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

Conversation

Projects
None yet
4 participants
@Findus23
Copy link
Member

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

@tsteur tsteur added this to the 3.9.0 milestone Dec 23, 2018

@tsteur

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 23, 2018

@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

This comment has been minimized.

Copy link
Member

tsteur commented Dec 23, 2018

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

something like this should do 👍

@Findus23

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 23, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 25, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 25, 2018

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

@tsteur

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 25, 2018

@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

This comment has been minimized.

Copy link
Member

tsteur commented Dec 25, 2018

changed it @Findus23

@Findus23

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 27, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

diosmosis commented Jan 22, 2019

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

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@diosmosis diosmosis deleted the no-more-feedburner branch Jan 22, 2019

@mattab mattab modified the milestones: 3.9.0, 3.8.1 Jan 22, 2019

flyapen added a commit to ICTO/matomo that referenced this pull request Jan 23, 2019

improve RSS fetching (matomo-org#13904)
* improve RSS fetching

* add cache to rss fetching and remove "appeared first on" appendix

* remove special handling of RSS description

* Make sure links in rss content open in new tab and have noreferrer noopener.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment