980243 - Wrong display of OpenLayers GIS maps

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[hattori]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140303165517

Steps to reproduce:

1. Open http://mapa-banska-bystrica.gisplan.sk/mapa/
2. When the first layer loads, choose one of the layers listed in the left panel, e.g. "Ortofotomapa 2012"
3. After the layer loads, click the "refresh button" represented by two blue arrows going in a circle. It is located in the left button panel - second icon from the bottom.


Actual results:

The layers completely disappear.


Expected results:

The layers shoud refresh and display again. The same happens with all layers - raster and vector

Comment 1

[Alice0775 White]

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2390cb215cc4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20131010072645
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/3bc7ae253cdf
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20131010075449
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2390cb215cc4&tochange=3bc7ae253cdf
Regressed by:
	41e7e41afe1f	Mina Almasry — Bug 696451 - Reload <img> when crossOrigin attribute is set. r=bz

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.  This seems to be due to the "src set" logic moving from SetAttr to AfterSetAttr.

Interestingly, once we're in the broken state opening or closing devtools immediately fixes.  But just resizing the window does NOT fix.

Interestingly, removing the nodes from the DOM and then putting them back also doesn't fix.  So I'm not sure what the story is with the devtools bit there.

I'll try to poke at this a bit, but any information on what that "refresh" button is actually doing, or anything that creates a smaller testcase, would be very welcome!

Comment 3

[Boris Zbarsky [:bzbarsky]]

Actually, nevermind.  I still don't know exactly why it's being an issue, but the move of the src set to AfterSetAttr was clearly wrong.  Patch, with comments, coming up.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I bet the page is watching for load events that never happen or something....

Comment 5

[Boris Zbarsky [:bzbarsky]]

Though Chrome doesn't send load events for src sets to existing value.  So I'm really not sure what the deal is there.  :(

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure that image src sets do something even if the src is being set to the value it already has.

Jonas, since it's sort of your prompting that introduced the bug, you get to review the fix.  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

[e-mail], thank you for the bug report!

Comment 8

[Boris Zbarsky [:bzbarsky]]

> So I'm really not sure what the deal is there.  :(

And hence would still love a reduced-ish testcase.

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Today is the final beta for FF28 and we've already lived with this in FF27 for almost 6 weeks, will have to track for FF29 at the earliest and try to get this regression fixed in time for that release - ideally early in Beta 29 cycle.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8386827 [details] [diff] [review]
Make sure that image src sets do something even if the src is being set to the value it already has.

Review of attachment 8386827 [details] [diff] [review]:
-----------------------------------------------------------------

This seems like the right thing to do, though it's not clear to me why it fixes the bug?

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb77e0285652

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8386827 [details] [diff] [review]
Make sure that image src sets do something even if the src is being set to the value it already has.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 696451
User impact if declined: Some sites do not work correctly
Testing completed (on m-c, etc.): Passes tests
Risk to taking this patch (and alternatives if risky): Moderate risk, somewhat
   mitigated by this being a reversion to the previous behavior we had in Firefox
   26.  The obvious other option is to let this ride the trains.
String or IDL/UUID changes made by this patch: None.

I don't think we want a beta 28 backport at this point, but if someone thinks we do please request beta approval....

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bb77e0285652

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b77120159790