Page MenuHomePhabricator

[bug] related articles /sometimes/ appear in wrong place on articles
Closed, ResolvedPublic3 Estimated Story Points

Description

Steps to reproduce:

Bug:
related article block appears between Last edited and last section instead of being on the base layer. it looks broken as well.

It's been almost a year we deployed related pages, this was thought to be a caching issue but I still see this behaviour every now and then. anecdotally, once in three.

Feel free to close it if this is still a caching issue. but any explanation in why such issues are not seen with any other UI changes we do, will be helpful.

Developer notes

Update resources/ext.relatedArticles.readMore.bootstrap/init.js so that it only adds the read more widget to the page when the document is ready.

Testing criteria

Go through 30-40 articles and note their related pages section. Related pages should appear at the bottom of the page under "last edited".

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 15 2018, 5:59 PM

What was the URL? I've also seen this issue. If I recall correctly I noticed it was related to unclosed div's in the article HTML.

Jdlrobson renamed this task from [bug] related articles appear in wrong place on articles to [bug] related articles /sometimes/ appear in wrong place on articles.Feb 15 2018, 6:02 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.

If I recall correctly I noticed it was related to unclosed div's in the article HTML.

this issue gets fixed if you refresh the page. so it's probably not because of article structure

url is https://en.wikipedia.org/wiki/Digest_size

Have you seen this in any browser other than Chrome?

yes, happens on same article on safari as well

Jdlrobson updated the task description. (Show Details)Feb 15 2018, 6:21 PM
ovasileva triaged this task as Medium priority.Feb 15 2018, 6:22 PM

So this is how the content gets added to the page

if ( $( '.footer-content' ).length ) {
        $( '<div class="read-more-container" />' ).prependTo( '.footer-content' );
} else {
        $( '<div class="read-more-container post-content" />' )
                .insertAfter( '#content' );
}

So what's happening (for reasons unknown is $( '.footer-content' ).length is being evaluated as 0)
This suggests something weird is happening in the Minerva skin. This element should always exist.. unless the DOM hasn't been completed parsed when the code is run.

This is hopefully a simple case of wrapping this in a document ready.

Let's try that as a starting point?

ovasileva set the point value for this task to 3.Feb 20 2018, 5:16 PM
Niedzielski moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.

@ovasileva pulling this into the sprint from sprint+1 because all other tasks are claimed

Change 413193 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/RelatedArticles@master] Fix: wait to show related articles until ready

https://gerrit.wikimedia.org/r/413193

Niedzielski removed Niedzielski as the assignee of this task.Feb 21 2018, 4:24 PM
Niedzielski added a subscriber: Niedzielski.

This is not easy to replicate so I'm not sure if it's possible to easily QA this.

Change 413193 merged by jenkins-bot:
[mediawiki/extensions/RelatedArticles@master] Fix: wait to show related articles until ready

https://gerrit.wikimedia.org/r/413193

a weird QA method would be going to random articles and seeing the footer. the current frequency is very high. you would at least find 1/5 articles with broken footers. with this change, finding even one in 10-20 would mean we didn't fix it. all we need is one and it's easy to find. unless there are multiple issues causing this and we fixed one of them which I would find very fascinating

This can be tested on the beta cluster now. If we need production to do this, this will need to stay open until next week when the change goes live.

Let's wait until it's live since we'll have a larger sample to test on.

ovasileva added a subscriber: ABorbaWMF.

over to you @ABorbaWMF, this will be ready to test Thursday, March 1st. Testing criteria in the description

ovasileva updated the task description. (Show Details)Feb 26 2018, 6:09 PM

Hey @ABorbaWMF - I meant this will be ready to test on production Thursday, March 1st. Do you think you could go through a number of articles (maybe 20-30?) on production to confirm? This bug is a bit odd to catch, hence the need for more test articles.

Looking good on production as well. Tested on a variety of browsers/OSs.

ovasileva closed this task as Resolved.Mar 6 2018, 12:04 PM

Thanks @ABorbaWMF! Looks like we're all done here.