Page MenuHomePhabricator

Map code throwing JS exception on IE11 Windows 7 causing Pagebanner not to be shown and Common.js not running due to JS error thrown by leaflet
Closed, ResolvedPublic

Description

My Internet Explorer (version 11) shows correctly all the imaged but the pagebanner that is "blank". After few refresh and/or new tab opening, the banner appears.

While on fr:voy happens only the above issue, on it:voy there is a further side effect: when the banner is not shown the breadcrumb appear below the banner.
Tested pages: https://it.wikivoyage.org/wiki/Sardegna and https://fr.wikivoyage.org/wiki/Sardaigne

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Aklapper renamed this task from Pagebanner is not shown on Internet Explorer to Pagebanner not shown on IE11.Dec 2 2016, 9:54 AM
Jdlrobson added subscribers: ovasileva, Jdlrobson.

@ovasileva probably something we should fix sooner rather than later.

Jdlrobson lowered the priority of this task from High to Medium.Dec 13 2016, 10:28 PM

Hi @Andyrom75 I tried to replicate this on browserstack.com IE11 Windows 10 but couldn't - page banner shows fine for me. I can't see what's wrong. Can you clarify?
Does the issue happen when you are logged out?
Which Windows version are you using?

Screen Shot 2016-12-13 at 2.25.14 PM.png (559×1 px, 844 KB)

Screen Shot 2016-12-13 at 2.27.48 PM.png (452×1 px, 731 KB)

Tested on the final application version of IE 11 - I'm not sure of the exact reproduction instructions but on my machines they looked fine -

T152185 Sardaigne.png (899×1 px, 686 KB)

T152185 Sardegna.png (857×1 px, 794 KB)

Here below attached what I see on it:voy (jointly with my IE & Windows version).

pasted_file (738×1 px, 400 KB)

Regarding fr:voy this is what I see "sometimes" although after few refresh I've seen it regularly

pasted_file (738×1 px, 174 KB)

Note: when I see it regularly, it appears after a fraction of second. It's like it's shown, not like all the other images, but through a JS instruction, so I suppose that when it doesn't appear, there's something that block the JS execution. It's just a feeling because I haven't seen the code.

Oh I see. Thanks for the thorough report!
This is due to a workaround in https://fr.wikivoyage.org/w/index.php?title=MediaWiki:Common.css for bug T113642
I don't see that bug getting fixed anytime soon so it might be a good time to decide whether it matters if the breadcrumbs show below the page banner...

Breadcrumbs has been always shown above the banner, inline with the geo coords and map icons.
That is the less intrusive location because it optimizes the available space.

I understand why this is wanted, but the extension that provides the functionality is programmed to always show breadcrumbs below the title as discussed in T113642 and will need to be updated to fix that.

Maybe I'm wrong (because I repeat that I haven't seen the code) but I suppose that reason why the banner is shown above the breadcrumb could be the same reason why the banner is shown above the diff.
If confirmed, should be rethought how the banner is displayed.

PS Other users do not see regularly the banner on both it:voy and fr:voy with browsers different from Chrome

My pc now the situation is this:
pages viewed with IE browser and Google Chrome: banner not shown on it: Wv - banner shown on other Wv;
pages viewed with Firefox browser: banners shown on it: Wv and all other Wv.

Jdlrobson renamed this task from Pagebanner not shown on IE11 to Map code throwing JS exception on IE11 Windows 7 causing Pagebanner not to be shown and Common.js not running.Feb 10 2017, 7:30 PM
Jdlrobson edited projects, added Maps; removed Web-Team-Backlog, Wikidata-Page-Banner, Wikidata.

While on fr:voy happens only the above issue, on it:voy there is a further side effect: when the banner is not shown the breadcrumb appear below the banner.

There is a workaround for T113642 in MediaWiki:Common.js that moves the breadcrumb. The fact that you're seeing the breadcrumb below tells me that there is an issue with the JavaScript loading on your page.

A quick experiment and I can replicate this by hard refreshing the page with browser stack IE11 Windows 7.
I've done some debugging and this is not an issue with the banner but with the Leaflet library.

Unable to get property 'isArray' of undefined or null reference
File: leaflet.js, Line: 6, Column: 223

This JavaScript error is causing the problems you are seeing.

Any update?
Today an Italian article has been shown on TV (http://www.teleboario.it/tbnews/turismo-per-iseo-un-altro-riconoscimento-dal-web/) without the proper banner and honestly it's a bit embarassing for the whole project.

Aklapper renamed this task from Map code throwing JS exception on IE11 Windows 7 causing Pagebanner not to be shown and Common.js not running to Map code throwing JS exception on IE11 Windows 7 causing Pagebanner not to be shown and Common.js not running due to JS error thrown by leaflet.May 11 2018, 3:12 PM

Change 432618 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/WikidataPageBanner@master] Show banner that is loaded from cache

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

That's unfortunate :(
Thanks for bumping this ticket. It looks like the leaflet issue is fixed which makes this easier to debug.

I have literally no maintainers for this extension other than myself, but I think https://gerrit.wikimedia.org/r/432618 should fix this. I just don't know how to get that reviewed other than self merging. (If you know anyone to help out with volunteering please push them my way!!)

In mean time, try placing this in MediaWiki:Common.js to see if the issue will go away.

	var $wpbBannerImageContainer = $( '.wpb-topbanner' ),
		$img = $( 'img.wpb-banner-image' );

	if ( $img[0] && $img[0].complete ) {
		$wpbBannerImageContainer.addClass( 'wpb-positioned-banner' );
	}

@Jdlrobson, I've tried to implement the patch but it seems that it doesn't work. I'll keep it few days more in case you'll need it for a test.

@Andyrom75 maybe wrap in a $(function(){})
When you say it doesnt work do you mean the banner still fails to load? Are there any errors in the IE developer tools console?

@Jdlrobson, by now it seems that the patch works. IE11 doesn't send any error but several warnings most likely not related to this issue.
Since also other language versions should be affected by this problem, I think that the bug should be fixed soon.
Thanks by now.

Unfortunately the problem still happens but rarely :-(
It seems that no error has been generated.

Jdlrobson closed this task as Resolved.EditedJun 6 2018, 10:14 AM
Jdlrobson claimed this task.

I can't seem to replicate this on Italian or French Wikipedia any more. I'm fairly confident something in MediaWiki:Common.js or a gadget was interfering with the banner positioning code. Please reopen this if the issue comes back. If it does, please provide screen resolution information as possibly that is important here.

Jdlrobson added a project: RelatedArticles.
Jdlrobson added a subscriber: Krinkle.

Spoke too soon. I found a page - https://fr.wikivoyage.org/wiki/Trouville-sur-Mer at resolution 2048x1536

Again, it is caused by a JavaScript error outside the extension:

Exception in load-callback in module ext.relatedArticles.readMore:
[object Error]{description: "Object does...", message: "Object does...", name: "TypeError", number: -2146827850, stack: "TypeError: ..."}
Object doesn't support property or method 'remove'

I'm not sure why the banner code is not resilient to JavaScript errors - but it seems any time that a JavaScript error is thrown the following code inside the positionBanner code does not get run:

$img.load( function () {
		positionBanner( $wpbBannerImageContainer );
		$wpbBannerImageContainer.addClass( 'wpb-positioned-banner' );
	} );

@Krinkle any thoughts? I thought each module was supposed to be sandboxed?

Change 437716 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/RelatedArticles@master] Correctly remove node without exception

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

Change 437716 merged by jenkins-bot:
[mediawiki/extensions/RelatedArticles@master] Correctly remove node without exception

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

That should probably get rid of the remaining errors. Just need some thoughts from @Krinkle about whether there is any way we can future proof this, by making its JS more resilient to JavaScript errors from other extensions.

@Jdlrobson Yes, there is some form of sandboxing at the level of module loading. Which means that if you define module "a" and "b" as a.js and b.js, and load them separately but in the same request with "a" before "b", then even if "a" throws an exception during initialisation (e.g. syntax error or some other error that happens when first executing the file), then "b" should still run without issue.

Can you still reproduce a problem locally when RelatedArticles is without the above patch? If so, could you try to make a more minimal test case that ends up with e.g. putting a certain piece of code in module X that makes it fail and something in module Y, where Y won't get executed?

Change 432618 abandoned by Jdlrobson:
Show banner that is loaded from cache

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

Can you still reproduce a problem locally when RelatedArticles is without the above patch? If so, could you try to make a more minimal test case that ends up with e.g. putting a certain piece of code in module X that makes it fail and something in module Y, where Y won't get executed?

It looks like what's happening is WikidataPageBanner code runs and binds callback to load event; RelatedArticles throws the exception; Image loads but callback not invoked, but I have no idea how to narrow that down. I've spent a few hours now trying to work this out, but didn't get anywhere.

This bug is by fixed https://gerrit.wikimedia.org/r/437716 and I've made a follow up based on your advice - https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/RelatedArticles/+/437716/1/resources/ext.relatedArticles.readMore.bootstrap/index.js - let's continue the conversation about the image loading elsewhere if needed but this is low priority for web right now.