<input type="color"> crashes Firefox on OS X

VERIFIED FIXED in Firefox 49

Status

()

Core
Widget: Cocoa
--
critical
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: danburzo, Assigned: spohl)

Tracking

({crash, crashreportid})

48 Branch
mozilla51
crash, crashreportid
Points:
---

Firefox Tracking Flags

(firefox48 wontfix, firefox49 verified, firefox50 verified, firefox51 verified)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
strtestcase
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?
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 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)
status-firefox48: affected → wontfix
Crash Signature: [@ libobjc.A.dylib@0x14e9]
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+
(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
Flags: qe-verify+
Verified fixed FX 49b8, 50.0a2 (2016-09-01) OS X 10.11.
status-firefox49: fixed → verified
status-firefox50: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.