Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve RSS fetching #13904

Merged
merged 4 commits into from Jan 22, 2019
Merged

improve RSS fetching #13904

merged 4 commits into from Jan 22, 2019

Conversation

Findus23
Copy link
Member

@Findus23 Findus23 commented Dec 21, 2018

Fixes #5214

While we are on the topic of improving old widgets:

  • The Matomo RSS feed now includes a .screen-reader-text summary directly after Read More, which needs to be hidden
  • Always use HTTPS
    • it's better to fail than show possibly mitm-ed HTML
  • Don't use feedburner
    • this has the huge advantage of not sending data to Google
    • but this means many requests will reach wordpress, and the endpoint would probably need to be cached
    • speaking of caching: I think Matomo should also cache this data locally so that not every reload fetches the data freshly

Semi-related: https://matomo.org/faq/how-to/faq_99/ should also be removed, archived or updated.

BTW: Feedburner adds tracking-pixels to the feed, but only to the content, so Matomo isn't affected

@Findus23 Findus23 added c: Privacy For issues that impact or improve the privacy. Needs Review PRs that need a code review labels Dec 21, 2018
@tsteur tsteur added this to the 3.9.0 milestone Dec 23, 2018
@tsteur
Copy link
Member

tsteur commented Dec 23, 2018

@Findus23 any chance we could not show the last part of each article saying "... appeared first on ..."?

image

@Findus23
Copy link
Member Author

@tsteur That was the reason why I initially looked into the rss fetching.
The sentence is part of the RSS feed, but maybe it is enough to always remove all text after Read More.

@tsteur
Copy link
Member

tsteur commented Dec 23, 2018

#widgetRssWidgetrssPiwik .read-more-container+p {
    display: none;
}

something like this should do 👍

@Findus23
Copy link
Member Author

@tsteur It is unfortunately not in a special class, but removing it via PHP seems easy.

There should also be a working cache now.

@tsteur
Copy link
Member

tsteur commented Dec 23, 2018

@Findus23 " It is unfortunately not in a special class"

do you mean that the sentence doesn't have a class? Cause the selector should still work. At least when I tested it ... or is the problem in case it changes in the future?

@Findus23
Copy link
Member Author

@tsteur
What I meant, is that the HTML is structured like this:

<p>There’s no doubt about it, the free version of Google Analytics offers great value when it comes to making
    data-driven decisions for your business. But as your business starts to grow, so does the need for a more powerful
    web ... </p>
<p class="read-more-container">
    <a title="Why Matomo is a serious alternative to Google Analytics 360"
       class="read-more button"
       href="https://matomo.org/blog/2018/12/why-matomo-is-a-serious-alternative-to-google-analytics-360/#more-30938">
        Read More
        <span class="screen-reader-text">
            Why Matomo is a serious alternative to Google Analytics 360
        </span>
    </a>
</p>
<p>The post
    <a rel="nofollow"
       href="https://matomo.org/blog/2018/12/why-matomo-is-a-serious-alternative-to-google-analytics-360/">
        Why Matomo is a serious alternative to Google Analytics 360</a> appeared first on
    <a rel="nofollow" href="https://matomo.org">Analytics
        Platform - Matomo</a>.
</p>

So by hiding .screen-reader-text, I can get rid of the screenreader summary, but not the last paragraph. So removing it via PHP (as I am doing now) is probably easier.

@tsteur
Copy link
Member

tsteur commented Dec 25, 2018

Did you try this?

#widgetRssWidgetrssPiwik .read-more-container+p {
    display: none;
}

it should select it or am I not understanding it?

@Findus23
Copy link
Member Author

@tsteur That should also work. Do you think this is less prone to break in the future?

@tsteur
Copy link
Member

tsteur commented Dec 25, 2018

Not 100% sure which is better. Do we know the feed is always retrieved in English? If the wording changes it might break vs CSS will still work. Both have advantages and disadvantages really.

@Findus23
Copy link
Member Author

@tsteur Of course there is the even better solution: Removing the extra text that seems to be added by
Yoast SEO.
https://www.feedbolt.com/why-do-my-posts-say-the-post-appeared-first/

@tsteur
Copy link
Member

tsteur commented Dec 25, 2018

changed it @Findus23

@Findus23
Copy link
Member Author

@tsteur Thanks I removed the PHP code and it seems to work fine.
Can you please check if the Cache is implemented correctly and if https://matomo.org/feed is cached enough to be able to handle the new requests.

@tsteur
Copy link
Member

tsteur commented Dec 27, 2018

Not sure about that, probably not. I reckon we would need to put this behind our CDN but https://static.matomo.org/feed/ currently redirects to matomo.org and would need to figure out how to disable this redirect in wordpress.

@diosmosis
Copy link
Member

Noticed links in rss content didn't have _target=blank or rel="noreferrer noopener", so added that. Looks good and works for me locally.

@diosmosis diosmosis merged commit fb633f5 into 3.x-dev Jan 22, 2019
@diosmosis diosmosis deleted the no-more-feedburner branch January 22, 2019 05:16
@mattab mattab modified the milestones: 3.9.0, 3.8.1 Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Privacy For issues that impact or improve the privacy. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants