Closed Bug 1141260 Opened 5 years ago Closed 5 years ago
Crash [@ mozilla::dom::Responsive
Image Selector::Content() ]
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
Looks like we might be in responsive mode but mResponsiveSelector is null?
Crash Signature: https://crash-stats.mozilla.com/report/index/4b40ceee-e4db-4f17-9e3f-283ca2150309
Duplicate of this bug: 1141278
Severity: normal → critical
Crash Signature: [@ mozilla::dom::ResponsiveImageSelector::Content() ] [@ mozilla::dom::HTMLImageElement::AfterSetAttr(int, nsIAtom*, nsAttrValue const*, bool) ]
OS: Mac OS X → All
Summary: [@ mozilla::dom::ResponsiveImageSelector::Content() ] → Crash [@ mozilla::dom::ResponsiveImageSelector::Content() ]
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
Duplicate of this bug: 1141322
[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
Version: unspecified → 38 Branch
There's lots of prior art for checks like this in the file.
Yeah the null check here is proper, and an oversight from when I added InResponsiveMode(), previously that line would've been | if (mResponsiveSelector) |.
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.
Duplicate of this bug: 1141664
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.
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.
Further reduction less than 1000 lines. Removed some JS stuff.
Attachment #8575692 - Attachment is obsolete: true
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.
It's caused by unsetting src when InResponsiveMode() returns true Crash test: > <img src="foo" srcset="bar"> > <script> > document.querySelector("img").removeAttribute('src'); > </script>
Ah, thanks. I apparently misread the stack and my tests for _setting_ the attribute weren't working.
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.
Attachment #8575024 - Flags: approval-mozilla-aurora?
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.
Attachment #8575024 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This appears to be fixed on 38 and 39 now. I don't think we need to keep tracking it.
You need to log in before you can comment on or make changes to this bug.