1141260 - Crash [@ mozilla::dom::ResponsiveImageSelector::Content() ]

Status: RESOLVED FIXED

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

Description

[David Humphrey (:humph)]

No STR for this, sorry. I had a bunch of tabs open, and browser had been running all day.

Running Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Firefox/38.0

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Looks like we might be in responsive mode but mResponsiveSelector is null?

Comment 3

[Johannes Pfrang [:johnp]]

Adding URL that triggers this crash quite reliably.

e10s-report: bp-4b40ceee-e4db-4f17-9e3f-283ca2150309

non-e10s-report: bp-81f60204-93fa-4719-8ef3-67ce02150309

Comment 5

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e00ed1b014d0&tochange=2bb299df4c33

Suspect: Bug 1017875

Comment 6

[Josh Matthews [:jdm]]

Attachment: Null-check the responsive selector in images

Comment 7

[Josh Matthews [:jdm]]

There's lots of prior art for checks like this in the file.

Comment 8

[John Schoenick [:johns]]

Yeah the null check here is proper, and an oversight from when I added InResponsiveMode(), previously that line would've been | if (mResponsiveSelector) |.

Comment 9

[Karl Dubost💡 :karlcow]

http://www.theguardian.com/uk 
crashing on both Aurora 38.0a2 (2015-03-09) and Nightly 39.0a1 (2015-03-09)
Repeatedly. 

From the user perspective, the page loads and then crashes right away: the tab in nightly, the browser in Aurora.

Comment 11

[Benoit Girard (:BenWa)]

Reproducible on:
http://www.theguardian.com/world/2015/mar/10/ten-dead-in-apparent-helicopter-crash-in-argentina

Comment 12

[Josh Matthews [:jdm]]

I spent some time attempting to make a crashtest for this, but reducing the code on the guardian led me to  8000 lines of minified JS and one very important <aside class="js-related">. I'm not convinced it's worth my time to attempt to further reduce this, and I'll land it without a test.

Comment 13

[Josh Matthews [:jdm]]

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

Comment 14

[Karl Dubost💡 :karlcow]

Attachment: guardian.html

Josh, 


how about ~1300 lines.
1. I reduced the CSS to its minimum, so it would create the crash
2. I reduced the markup It's in the section #highlights that it is happening. I remove what was not necessary. I didn't go into details.
3. I didn't touch inside these two script elements but they are necessary to reproduce the crash.

So we are getting closer I guess.

Comment 15

[Karl Dubost💡 :karlcow]

Attachment: guardian.html

Further reduction less than 1000 lines. Removed some JS stuff.

Comment 16

[Josh Matthews [:jdm]]

In my reduction, I did not require any CSS, not any other markup besides the script elements and the <aside>. When I commented out the bootstrap.go(), the crash disappeared, but inlining the corresponding source and calling it manually did not make it reappear.

Comment 17

[John Schoenick [:johns]]

It's caused by unsetting src when InResponsiveMode() returns true

Crash test:

> <img src="foo" srcset="bar">
> <script>
>   document.querySelector("img").removeAttribute('src');
> </script>

Comment 18

[Carsten Book [:Tomcat]]

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

Comment 19

[Josh Matthews [:jdm]]

Ah, thanks. I apparently misread the stack and my tests for _setting_ the attribute weren't working.

Comment 20

[Josh Matthews [:jdm]]

Comment on attachment 8575024 [details] [diff] [review]
Null-check the responsive selector in images

Approval Request Comment
[Feature/regressing bug #]: bug 1067345
[User impact if declined]: Crashes on common websites.
[Describe test coverage new/current, TreeHerder]: crashtest in bug 1142032
[Risks and why]: Simplest possible crash fix.
[String/UUID change made/needed]: None.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8575024 [details] [diff] [review]
Null-check the responsive selector in images

Uplifting to aurora since it fixes a regression and seems low risk. 
The test should also get uplifted once it lands.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/77c29b7f2e7c

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

This appears to be fixed on 38 and 39 now. I don't think we need to keep tracking it.