Page MenuHomePhabricator

Image display problem on Firefox
Closed, ResolvedPublic

Description

See the discussion at https://en.wikivoyage.org/wiki/Wikivoyage:Travellers%27_pub#banner_not_displaying. If you browse the site on Firefox, occasionally when an image is not in the cache the banner displays only partially.

In every case where I've been able to reproduce this bug I've seen a negative "margin-top" style added to the image, and that appears to come from the positionBanner() Javascript method. So far as I can tell, since the banner image width and height are not specified in the HTML, the postitionBanner() method should only be called once the banner image has downloaded, NOT on document.ready() as it currently is. Thus, I think that changing ext.WikidataPageBanner.positionBanner.js to only invoke positionBanner when the image has loaded might fix the problem:

$( "img.wpb-banner-image" ).load(function() {
positionBanner( $wpbBannerImageContainer );
});

I'm not 100% positive that this change will fix the issue, but based on the fact that the Javascript currently seems to be getting width/height calculations wrong only in cases where the image is not cached it makes sense conceptually that it would fix things.

Event Timeline

Wrh2 created this task.Sep 7 2015, 5:38 PM
Wrh2 raised the priority of this task from to Needs Triage.
Wrh2 updated the task description. (Show Details)
Wrh2 added a project: Wikidata-Page-Banner.
Wrh2 added subscribers: Wrh2, Jdlrobson.
Restricted Application added a project: Wikidata. · View Herald TranscriptSep 7 2015, 5:38 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Jdlrobson triaged this task as High priority.Sep 8 2015, 1:17 AM
Jdlrobson added a subscriber: Sumit.

Thanks for raising this. I too recall seeing something similar but thought it was just a caching issue. I suspect you are right that the banner is resizing before the image has fully loaded.
@Sumit will you be able to take a look at this or do you want me to have a go?

@Wrh2 do you want to add this code to MediaWiki:Mobile.css in the mean time to see if the problem goes away?

$( "img.wpb-banner-image" ).load(function() {
  mw.wpb.positionBanner( $( '.wpb-topbanner' ) );
} );
Wrh2 added a comment.EditedSep 8 2015, 1:58 AM

@Jdlrobson I tried the code you provided in my local common.js, and I also tried executing it on window.load (since the JQuery load event handler can be flaky) but was still able to reproduce the issue.

This might not be a great workaround, but if I'm understanding the purpose of the positionBanner() method correctly, the method can be modified to exit immediately if both of the following are zero, correct?

centerX = $wpbBannerImage.data( 'pos-x' ),
centerY = $wpbBannerImage.data( 'pos-y' );

The bug will still be there for banners that use these parameters, but since those are a tiny subset of all banners it should at least resolve the problem until a better fix is found.

Change 236735 had a related patch set uploaded (by Sumit):
WikidataPageBanner set focus after banner load

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

Sumit added a comment.Sep 8 2015, 6:42 AM

@Jdlrobson, I've added a fix, but since I'm unable to see the issue locally, I'll need your support for testing.
@Wrh2, I've tried testing the fix on Wikivoyage, by adding the code at https://en.wikivoyage.org/wiki/User:Sumit.iitp/common.js It seems to work and I've added the above patch along those lines. You can copy the same code to your User's common.js and verify if my assumption is true. It will help in attesting the correctness of the patch :)

Lydia_Pintscher moved this task from incoming to monitoring on the Wikidata board.Sep 8 2015, 9:36 AM
Wrh2 added a comment.Sep 8 2015, 3:12 PM

@Sumit - see my second comment on this ticket. I've tried updating my common.js file with the Javascript Jdlrobson suggested, as well as the Javascript you provided, but in both cases I'm still able to reproduce the problem. It appears that either Firefox is firing the onload event before the image is loaded or else JQuery is failing at a height calculation early in the load process.

If I disable Javascript then everything seems to work fine, so this seems to be a problem specific to the positionBanner() method. Am I correct in the belief that the CSS this method appends is only needed when the "pos-x" and "pos-y" values are non-zero? If so I think the method can exit without changing any CSS if those values are zero, which should be a good temporary fix for the majority of pages while the underlying issue is addressed.

Change 236735 merged by jenkins-bot:
WikidataPageBanner set focus after banner load

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

Sumit added a comment.Sep 10 2015, 3:54 AM

@Wrh2, the above fix has been deployed on English Wikivoyage. To test, I tried repeated refreshing after cache clearing each time on https://en.wikivoyage.org/wiki/Bertha_Benz_Memorial_Route. However I wasn't able to reproduce the issue. If the issue still persists, let me know the pages on which I can reproduce and see. Otherwise, I'll close this task :)

Wrh2 added a comment.Sep 10 2015, 4:58 AM

I was unable to reproduce the issue just now, but have also asked for feedback from the original reporters at https://en.wikivoyage.org/wiki/Wikivoyage:Travellers'_pub#banner_not_displaying.

Thanks for the quick attention.

Wrh2 added a comment.Sep 10 2015, 3:19 PM

The individual who originally reported this issue now also says it is fixed. Thanks again Sumit - this task can now be closed.

Jdlrobson closed this task as Resolved.Sep 10 2015, 3:30 PM
Jdlrobson claimed this task.

Awesome.