Status
()
People
(Reporter: danburzo, Assigned: spohl)
Tracking
({crash, crashreportid})
Firefox Tracking Flags
(firefox48 wontfix, firefox49 verified, firefox50 verified, firefox51 verified)
Details
(crash signature)
Attachments
(1 attachment)
1.82 KB,
patch
|
mstange
:
review+
sylvestre
:
approval-mozilla-aurora+
sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0 Build ID: 20160726073904 Steps to reproduce: User agent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0" OS X version: 10.11.6 1. Start with a HTML page that simply contains an <input type='color'> 2. Click on the color picker and in the OS X color picker switch to the "Color Sliders" tab 3. Try to drag the Brightness knob, repeatedly JSFiddle for convenience: https://jsfiddle.net/wp25dr9d/ Actual results: Browser crashes
Comment 1•2 years ago
|
||
bp-dc9f70d7-15cc-41d4-9f42-118ad2160820 The bug is in Cocoa?
Severity: normal → critical
Status: UNCONFIRMED → NEW
Component: Layout: Form Controls → Widget: Cocoa
Ever confirmed: true
Keywords: crash, crashreportid
(Assignee) | ||
Comment 2•2 years ago
|
||
Able to reproduce and I'm pretty confident I know what's going on. Waiting for clobber build to complete to confirm the patch.
Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
(Assignee) | ||
Comment 3•2 years ago
|
||
Created attachment 8783151 [details] [diff] [review] Patch In the steps to reproduce in comment 0, note that you may have to select "Gray Scale Slider" (or similar) between steps 2 and 3 from the drop down menu. I've been able to crash Firefox with "Gray Scale Slider" selected and then confirmed that this patch fixes the issue for all options in the drop down menu. This crash occurs because [NSColor getRed:green:blue:alpha] throws an (uncaught) NSException* when called on an NSColor instance with an incompatible NSColorSpace (see "Class at a Glance" at [1]). Note that we can't use |respondsToSelector:| to determine whether or not it will throw an exception. I moved the conversion of the color space from |nsColorPicker::Update| to |nsColorPicker::GetHexStringFromNSColor| to get it closer to the call to [NSColor getRed:green:blue:alpha] and make it easier to catch and handle the exception if one occurs. Side note: In bug 1247373, after doing the color conversion, we accidentally continued to pass |aColor| instead of |color| to |nsColorPicker::GetHexStringFromNSColor|. This crash would still have occurred though, since the color space with the "Gray Scale Slider" selected is NSCalibratedWhiteColorSpace. [1] https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSColor_Class/
Attachment #8783151 -
Flags: review?(mstange)
(Assignee) | ||
Updated•2 years ago
|
See Also: → bug 1247373
Comment 4•2 years ago
|
||
Comment on attachment 8783151 [details] [diff] [review] Patch Review of attachment 8783151 [details] [diff] [review]: ----------------------------------------------------------------- Oops!
Attachment #8783151 -
Flags: review?(mstange) → review+
(Assignee) | ||
Comment 5•2 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/66d066d5f608c969d486c310d297d2c7fac79aaa Bug 1293593: Fix crash due to an Objective-C exception when calling getRed:green:blue:alpha on an NSColor instance with an incompatible NSColorSpace. r=mstange
(Assignee) | ||
Updated•2 years ago
|
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox50: --- → affected
Comment 6•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/66d066d5f608
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment 7•2 years ago
|
||
Hello Stephen, could you please nominate for uplift to Aurora50 and Beta49?
Flags: needinfo?(spohl.mozilla.bugs)
(Assignee) | ||
Comment 8•2 years ago
|
||
Comment on attachment 8783151 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: n/a [User impact if declined]: Using the color picker on OSX may crash the browser. [Describe test coverage new/current, TreeHerder]: Verified locally. [Risks and why]: Virtually none. We add a try/catch block to catch a native exception that previously crashed the browser. All instances that didn't cause an exception continue to work as before. [String/UUID change made/needed]: none
Flags: needinfo?(spohl.mozilla.bugs)
Attachment #8783151 -
Flags: approval-mozilla-beta?
Attachment #8783151 -
Flags: approval-mozilla-aurora?
Comment 9•2 years ago
|
||
Hello Dan, could you please verify this issue is fixed as expected on Nightly build 8/23/2016? Thanks!
Flags: needinfo?(danburzo)
(Reporter) | ||
Comment 10•2 years ago
|
||
I can confirm this is now working as expected on Firefox Nightly. Thank you, everyone!
Flags: needinfo?(danburzo)
Updated•2 years ago
|
status-firefox48: affected → wontfix
Updated•2 years ago
|
Crash Signature: [@ libobjc.A.dylib@0x14e9]
Comment 11•2 years ago
|
||
Comment on attachment 8783151 [details] [diff] [review] Patch Fix a crash, taking it. Should be in 49 beta 7
Attachment #8783151 -
Flags: approval-mozilla-beta?
Attachment #8783151 -
Flags: approval-mozilla-beta+
Attachment #8783151 -
Flags: approval-mozilla-aurora?
Attachment #8783151 -
Flags: approval-mozilla-aurora+
Comment 12•2 years ago
|
||
(In reply to danburzo from comment #10) > I can confirm this is now working as expected on Firefox Nightly. Thank you, > everyone! Fantastic! Thanks a lot for a prompt follow up.
Status: RESOLVED → VERIFIED
status-firefox51: fixed → verified
Comment 13•2 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/41d9edca6780
status-firefox50: affected → fixed
Comment 14•2 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/c56047b7ac0d
status-firefox49: affected → fixed
Updated•2 years ago
|
Flags: qe-verify+
Comment 15•2 years ago
|
||
Verified fixed FX 49b8, 50.0a2 (2016-09-01) OS X 10.11.
status-firefox49: fixed → verified
status-firefox50: fixed → verified
Updated•2 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•