Last Comment Bug 646157 - Stops responding when selecting a checkbox on the Netapp admin page inside deeply nested <label>s
: Stops responding when selecting a checkbox on the Netapp admin page inside de...
Status: VERIFIED FIXED
: dev-doc-complete, hang, testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla8
Assigned To: Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
:
: Andrew Overholt [:overholt]
Mentors:
: 579352 667005 668534 (view as bug list)
Depends on: 673859
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-29 11:53 PDT by jason
Modified: 2011-12-28 09:31 PST (History)
16 users (show)
khuey: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Mini Dumpfile from Firefox (1.69 MB, application/octet-stream)
2011-03-29 12:01 PDT, jason
no flags Details
Stack Dump (159.98 KB, text/plain)
2011-03-30 10:23 PDT, jason
no flags Details
html causing problems (35.11 KB, text/html)
2011-03-30 11:28 PDT, jason
no flags Details
Patch v1 (3.43 KB, patch)
2011-04-07 13:31 PDT, Mounir Lamouri (:mounir)
bugs: review-
Details | Diff | Splinter Review
testcase (1.01 KB, text/html)
2011-04-07 13:36 PDT, Mounir Lamouri (:mounir)
no flags Details
testcase2 (1.07 KB, text/html)
2011-04-19 06:50 PDT, Olli Pettay [:smaug] (review request backlog because of a work week)
no flags Details
First half (10.86 KB, patch)
2011-07-18 16:45 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bugs: review+
Details | Diff | Splinter Review
Second half (6.77 KB, patch)
2011-07-18 17:16 PDT, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
bugs: review+
Details | Diff | Splinter Review

Description User image jason 2011-03-29 11:53:37 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0

It seems when selecting checkboxes on the Netapp Admin pages, seems to cause Firefox 4 to go into an infinity loop, where it refuses to respond.  This also occurs in Safemode, and in a new profile in Safe Mode.

In Process Explorer, it seems to run the CPU to 100% (of at least 1 processor), and the memory will climb for a few minutes upto around 300Mb then it will drop by around 100Mb and this will repeat.

Reproducible: Always

Steps to Reproduce:
1. Load Page to Manage Quotas
2. Click on Checkbox
3. Wait for a long time, it doesn't seem to crash or anything.
Actual Results:  
Locks up Firefox

Expected Results:  
It should check the box

I don't think there is anything at the moment.  Though limits the usefulness
Comment 1 User image jason 2011-03-29 12:01:05 PDT
Created attachment 522757 [details]
Mini Dumpfile from Firefox
Comment 2 User image jason 2011-03-29 12:03:13 PDT
I have also seen this in the netapp communities as well
http://communities.netapp.com/thread/13891
Comment 3 User image Matthias Versen [:Matti] 2011-03-29 14:46:38 PDT
Please create a stack trace with Windbg
https://developer.mozilla.org/en/How_to_get_a_stacktrace_with_WinDbg
Comment 4 User image (mostly gone) XtC4UaLL [:xtc4uall] 2011-03-30 08:58:28 PDT
Does it work in Fx 3.6? Does it work in one of the past Fx4 Betas?
Comment 5 User image jason 2011-03-30 09:47:46 PDT
it did work in 3.6, but I never tried in the Betas if I recall, as I had not required to manage the device in question, while I was using the Betas
Comment 6 User image jason 2011-03-30 10:23:26 PDT
Created attachment 523045 [details]
Stack Dump
Comment 7 User image Matthias Versen [:Matti] 2011-03-30 10:54:15 PDT
This seems to be the loop:
xul!nsHTMLLabelElement::PostHandleEvent
xul!nsEventTargetChainItem::HandleEventTargetChain
xul!nsEventTargetChainItem::HandleEventTargetChain
xul!nsEventDispatcher::Dispatch
xul!PresShell::HandleDOMEventWithTarget
xul!nsGenericElement::DispatchEvent
xul!nsGenericElement::DispatchClickEvent
xul!nsHTMLLabelElement::PostHandleEvent
xul!nsEventTargetChainItem::HandleEventTargetChain

This will be still difficult without a testcase and I'm really lost if it comes to select a component for this. Can you help here Boris ?
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-30 11:02:10 PDT
CCing some more folks.

The description doesn't sound like infinite recursion (e.g. no crash).  So what's really going on?

Reporter, are you able to attach the relevant HTML to this bug?
Comment 9 User image jason 2011-03-30 11:05:39 PDT
possibly but it does have to get sanitized heavily, since it deals with production stuff.   You may be able to contact netapp, and ask for their Simulator.
Comment 10 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-30 11:13:06 PDT
jason, is the concern about making the HTML completely public, or about anyone at all being able to see it?

That is, are you happier sending the HTML to me by private mail, with the promise that I would not be posting it anywhere?

Chris, do we have contacts at netapp?
Comment 11 User image jason 2011-03-30 11:15:46 PDT
it seems that there are a large number of 
<label for="vol-1" /><input name="vol-13" type="hidden" value="V00067" title="vol-13" id="vol-1" />

type lines, at 10 it is very slow, at 20 it stops responding.
Comment 12 User image jason 2011-03-30 11:28:23 PDT
Created attachment 523068 [details]
html causing problems

it has been slightly santized.
Comment 13 User image jason 2011-03-30 11:30:08 PDT
Comment on attachment 523068 [details]
html causing problems

Even locally this still causes problems, and with javascript disabled as well.
Comment 14 User image jason 2011-03-30 11:47:47 PDT
At 20 <label> tags, it stops responding while it's processing, and takes around 5 minutes or more to start responding.

Doing some tests, it looks like the following
13 - 3 seconds
14 - 6 seconds
16 - 10 seconds or so
Comment 15 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-30 12:18:49 PDT
Ah, so tons of nested labels, and then you click something inside the innermost one.  I see.

In firefox 3.6 the labels do NOT nest.
Comment 16 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-30 12:20:47 PDT
They do nest in Opera and Webkit, though.

Are we ending up with something silly like activating labels exponentially many times in the depth of nesting?
Comment 17 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-30 12:21:11 PDT
Oh, http://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Clabel%2F%3E%0A%3Clabel%2F%3E for the testcase for parser behavior.
Comment 18 User image jason 2011-03-30 12:28:55 PDT
Actually even clicking on the first item in the list triggers this.
Comment 19 User image Mounir Lamouri (:mounir) 2011-04-07 13:31:02 PDT
Created attachment 524468 [details] [diff] [review]
Patch v1

Actually, label elements ignore click events coming from their descendants but the code doing that go from the target of the event to the label but doesn't check siblings, only parents.

The behavior we have with this patch seems to be the same on Opera. I do not have any webkit browser on this laptop.
Comment 20 User image Mounir Lamouri (:mounir) 2011-04-07 13:36:45 PDT
Created attachment 524470 [details]
testcase

If someone can give me the results when clicking on the rightest label (with various browsers): it should be 1/6.
Comment 21 User image Boris Zbarsky [:bz] (still a bit busy) 2011-04-07 14:10:26 PDT
It's 1/6 in Safari, Chrome, and Opera over here.
Comment 22 User image Mounir Lamouri (:mounir) 2011-04-07 14:52:25 PDT
Nice! Thanks for checking :)
Comment 23 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2011-04-18 07:45:02 PDT
Comment on attachment 524468 [details] [diff] [review]
Patch v1

I don't understand why you check only previoussibling and not nextsibling.
Comment 24 User image Mounir Lamouri (:mounir) 2011-04-18 08:07:53 PDT
(In reply to comment #23)
> Comment on attachment 524468 [details] [diff] [review]
> Patch v1
> 
> I don't understand why you check only previoussibling and not nextsibling.

This code tries to prevent sending a click event to the control element if it has been sent to it already. AFAIK, this can't happen if the element is after the target element in tree order. IOW, the method reproduces how the event bubbles and make sure that the control element isn't in the bubbling path.
Comment 25 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2011-04-19 06:50:55 PDT
Created attachment 526976 [details]
testcase2

With or without the patch our behavior when clicking the last checkbox is different than on Opera or Chromium
Comment 26 User image Olli Pettay [:smaug] (review request backlog because of a work week) 2011-04-19 06:57:13 PDT
Though, actually, I didn't check that Opera or Chromium parse the testcase
the same way as gecko.
Comment 27 User image Luis 2011-05-06 00:22:02 PDT
Setting "html5.parse.enable" to false seems to solve the problem. See bug 579352.
Comment 28 User image Henri Sivonen (:hsivonen) 2011-05-15 06:32:43 PDT
*** Bug 579352 has been marked as a duplicate of this bug. ***
Comment 29 User image jason 2011-06-17 07:46:27 PDT
is there any update for when this patch will be included?
Comment 30 User image Matthias Versen [:Matti] 2011-06-17 11:01:43 PDT
The patch got a negative review and it will be not included in any Firefox release. When the next patch gets a positive review it will get checked into the trunk and that's currently FF7.
Comment 31 User image Boris Zbarsky [:bz] (still a bit busy) 2011-06-27 13:04:07 PDT
*** Bug 667005 has been marked as a duplicate of this bug. ***
Comment 32 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-27 13:08:09 PDT
Mounir, what is the status here?
Comment 33 User image Boris Zbarsky [:bz] (still a bit busy) 2011-06-30 10:49:07 PDT
*** Bug 668534 has been marked as a duplicate of this bug. ***
Comment 34 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-12 17:36:38 PDT
I investigated this a bit.  The behaviors in various browsers are:

 Engine |  A bubbling click can trigger  |  A label-generated click can
        |  multiple <labels>             |  trigger additional labels
--------+--------------------------------+----------------------------------------
 Gecko  |           Yes                  |              Yes
--------+--------------------------------+----------------------------------------
 Webkit |                                |
 / IE   |           No                   |              No
--------+--------------------------------+----------------------------------------
 Opera  |           No                   |              Yes

Note that Opera and Webkit agree on Mounir's testcase if you click on the checkbox, but disagree if you click on the text, because of their differences here.
Comment 35 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-12 18:00:48 PDT
In fact, no other browser propagates a click event beyond the first label it encounters.
Comment 36 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-18 09:44:07 PDT
So the spec doesn't really say what to do here.

"The label element's exact default presentation and behavior, in particular what its activation behavior might be, if anything, should match the platform's label behavior."

As far as I know, platforms don't really have the concept of "bubbling" events ...
Comment 37 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-18 10:42:16 PDT
Olli says we should copy the WebKit/IE behavior here.
Comment 38 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-18 14:36:51 PDT
(In reply to comment #37)
> Olli says we should copy the WebKit/IE behavior here.

except that we're going to let the original click continue to bubble.
Comment 39 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-18 16:45:33 PDT
Created attachment 546668 [details] [diff] [review]
First half

This is the first half.  This patch stops a bubbling click from triggering multiple <label>s.  This is enough to fix the hang.
Comment 40 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-18 17:16:01 PDT
Created attachment 546683 [details] [diff] [review]
Second half

Propagate the "I've trigger a label" flag to the synthetic click event.

I'm going to throw this whole thing at try to see if anything depends on our existing behavior.
Comment 41 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-18 17:35:01 PDT
After applying these two patches we should behave like WebKit and IE except that we will allow the original click event to continue to bubble.
Comment 42 User image Mozilla RelEng Bot 2011-07-18 22:41:00 PDT
Try run for e094d7112f16 is complete.
Detailed breakdown of the results available here:
    http://tbpl.mozilla.org/?tree=Try&rev=e094d7112f16
Results:
    success: 146
    warnings: 15
    failure: 2
Total buildrequests: 163
Comment 43 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-22 06:58:24 PDT
http://hg.mozilla.org/mozilla-central/rev/09b2569edec2

The behaviors that I describe in the commit message are worth documenting on MDC, I think.
Comment 44 User image Jake Maul [:jakem] 2011-07-22 11:47:54 PDT
Pressing my luck here, but any chance this could land in Aurora (Fx7) as well? :)
Comment 45 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-25 13:54:43 PDT
(In reply to comment #44)
> Pressing my luck here, but any chance this could land in Aurora (Fx7) as
> well? :)

Don't think so, sorry :-(
Comment 46 User image Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-07-25 13:55:55 PDT
If you wanted to verify that this is fixed on Nightlies, that would be awesome :-)
Comment 47 User image Jake Maul [:jakem] 2011-07-25 15:16:48 PDT
The NetApp FilerView hang is indeed fixed in the 7/25 nightly build. :) :) :)
Comment 48 User image Shyam Mani [:fox2mike] 2011-07-25 16:59:53 PDT
Kyle,

Just a note of thanks from both myself and Jake, for working on this, pushing this to the right component and getting it looked at, testing, working on patches etc.

Thanks again!

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