Last Comment Bug 720058 - Crash when tapping multiple comboboxes, then dismissing multiple dialogs
: Crash when tapping multiple comboboxes, then dismissing multiple dialogs
Status: VERIFIED FIXED
[native-crash]
: crash, testcase
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: P2 critical (vote)
: Firefox 12
Assigned To: Wesley Johnston (:wesj)
:
Mentors:
Depends on:
Blocks: 707029
  Show dependency treegraph
 
Reported: 2012-01-20 18:08 PST by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2012-01-27 11:01 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
testcase (365 bytes, text/html)
2012-01-20 18:08 PST, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Catlog of crash (31.47 KB, text/plain)
2012-01-20 18:52 PST, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Patch (2.28 KB, patch)
2012-01-24 11:35 PST, Wesley Johnston (:wesj)
bnicholson: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2012-01-20 18:08:19 PST
Created attachment 590402 [details]
testcase

Tested on the LG Optimus Black, Android 2.2.2.

Steps to reproduce:
- Open testcase, tap on multiple comboboxes quickly.

Multiple dialogs appear (should not happen, should it?). After tapping on 3 on them, Fennec hangs and after a while you get the "not responding" dialog, where you finally can choose to force close Fennec.
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-01-20 18:52:08 PST
Created attachment 590408 [details]
Catlog of crash
Comment 2 Wesley Johnston (:wesj) 2012-01-24 11:35:01 PST
Created attachment 591199 [details] [diff] [review]
Patch

This is really a dup of bug 707029, and I also fixed bug 718965 while I was here.
Comment 3 Brian Nicholson (:bnicholson) 2012-01-24 15:41:01 PST
Comment on attachment 591199 [details] [diff] [review]
Patch

       result.listitems[aIndex] = {
         label: aNode.text || aNode.label,
         isGroup: this._isOptionGroupElement(aNode),
         inGroup: this._isOptionGroupElement(aNode.parentNode),
         disabled: aNode.disabled,
         id: aIndex
       }
+      if (result.listitems[aIndex].inGroup)
+        result.listitems[aIndex].disabled = result.listitems[aIndex].disabled || aNode.parentNode.disabled;

It might be a bit cleaner if you factor out result.listitems[aIndex]:
       let listItem = result.listitems[aIndex] = {
         label: aNode.text || aNode.label,
         isGroup: this._isOptionGroupElement(aNode),
         inGroup: this._isOptionGroupElement(aNode.parentNode),
         disabled: aNode.disabled,
         id: aIndex
       };
       if (listItem.inGroup)
         listItem.disabled = listItem.disabled || aNode.parentNode.disabled;
Comment 5 Wesley Johnston (:wesj) 2012-01-24 17:35:33 PST
Whoops brian. Completely forgot about the comment idea. Fixed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80a63d2f69ea
Comment 7 Wesley Johnston (:wesj) 2012-01-25 09:50:05 PST
Comment on attachment 591199 [details] [diff] [review]
Patch

[Approval Request Comment]
Regression caused by (bug #): bug 695485
User impact if declined: potential crashiness on a few sites
Testing completed (on m-c, etc.): landed on Jan 24
Risk to taking this patch (and alternatives if risky): low risk.
Comment 8 Martijn Wargers [:mwargers] (not working for Mozilla) 2012-01-25 12:20:26 PST
Verified fixed in current Native trunk build.
Comment 9 Alex Keybl [:akeybl] 2012-01-25 17:07:33 PST
Comment on attachment 591199 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Aurora.
Comment 10 Matt Brubeck (:mbrubeck) 2012-01-26 17:35:04 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/20e91ae263ad

Note You need to log in before you can comment on or make changes to this bug.