Closed Bug 1293593 Opened 4 years ago Closed 4 years ago
<input type="color"> crashes Firefox on OS X
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
bp-dc9f70d7-15cc-41d4-9f42-118ad2160820 The bug is in Cocoa?
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
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 ). 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.  https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSColor_Class/
Attachment #8783151 - Flags: review?(mstange)
Comment on attachment 8783151 [details] [diff] [review] Patch Review of attachment 8783151 [details] [diff] [review]: ----------------------------------------------------------------- Oops!
Attachment #8783151 - Flags: review?(mstange) → review+
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
Hello Stephen, could you please nominate for uplift to Aurora50 and Beta49?
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
Hello Dan, could you please verify this issue is fixed as expected on Nightly build 8/23/2016? Thanks!
I can confirm this is now working as expected on Firefox Nightly. Thank you, everyone!
Comment on attachment 8783151 [details] [diff] [review] Patch Fix a crash, taking it. Should be in 49 beta 7
(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.
Verified fixed FX 49b8, 50.0a2 (2016-09-01) OS X 10.11.
You need to log in before you can comment on or make changes to this bug.