Closed
Bug 449076
Opened 16 years ago
Closed 16 years ago
hang with 100% CPU on keyboard arrow trough text field
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Core
DOM: UI Events & Focus Handling
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b2
People
(Reporter: erwin, Assigned: uriber)
References
()
Details
(Keywords: hang, regression, testcase)
Attachments
(4 files)
337 bytes,
text/html
|
Details | |
5.05 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
976 bytes,
patch
|
Details | Diff | Splinter Review | |
2.97 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; nl; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; nl; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 I have made some text in my reaction form, when I tried to reset the cursur somewhere in the text nothing happened, when I used the arrow keys on my keyboard firefox freezes. This only happens in the text form with "Reactie:" next to it (see URL, Tip: use arrow keys on my site to scroll down to reaction form!) Reproducible: Always Steps to Reproduce: 1. Scroll to bottom (reaction form). 2. Type some text in the text form (next to "Reactie:". (optionel) 3. Try to put the cursor somewhere in the text (mouse). 4. Nothing happens, so try it with the arrow keys (keyboard) Actual Results: Firefox freezes Expected Results: put the cursor somewher in the field (specified place)
Comment 1•16 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 ID:2008070208 confirmed with slight modification to STR: 1. load URL 2. scroll to bottom of frame with scroll arrows on RHS 3. click in text field labeled "Reactie:" 4. Press down arrow key followed by up arrow key Actual results: Hang with 100% CPU marking qawanted for minimised testcase
Status: UNCONFIRMED → NEW
Component: General → Event Handling
Ever confirmed: true
Product: Firefox → Core
Summary: crash on keyboard arrow trough text field → hang with 100% CPU on keyboard arrow trough text field
Version: unspecified → 1.9.0 Branch
Comment 2•16 years ago
|
||
This affects the Trunk too, reproduced using the steps from C#1 with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008080400 Mnenhy/0.7.5.20005 SeaMonkey/2.0a1pre
Comment 3•16 years ago
|
||
Combination of '-moz-user-select: none' on the body, 'clear: left' and input + textarea required. Similar str: 1) load testcase 2) click mouse in textarea 3) Press up arrow followed by down arrow
Updated•16 years ago
|
Comment 4•16 years ago
|
||
Works: 2006083004 Fails: 2006083009 http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2006-08-30+03%3A00&maxdate=2006-08-30+10%3A00 I guess from bug 334626.
Blocks: 334626
Keywords: regression
Comment 5•16 years ago
|
||
Some additional details which may be helpful: replacing the <input> in the testcase with a <textarea></textarea> also hangs (with caret in either textarea) I looked into the hang and found that the do loop around calls to nsFrame::GetNextPrevLineFromeBlockFrame() never exits. See http://mxr.mozilla.org/firefox/source/layout/generic/nsFrame.cpp#4877 hth
Assignee | ||
Comment 6•16 years ago
|
||
Sorry for ignoring this for so long. This patch might not address the root cause of the hang, but it prevents it by preventing GetLineNumber() from breaking outside of a ScrollFrame, which I think is a good idea anyway. The behavior of up/down arrows here is still inconsistent with that of left/right (up/down don't allow the caret to be placed inside the unselectable text, whereas left/right do). I'm not sure what the semantics of -moz-user-select:none should be (or even what's the point of having editable-but-not-selectable content). The eSelectLine case in PeekOffset(), together with GetNextPrevLineFromeBlockFrame(), probably deserve a good rewrite.
Assignee | ||
Comment 7•16 years ago
|
||
This is a more direct fix to the problem: In our case, nsFrameIterator::CurrentItem was returning a null item and NS_ENUMERATOR_FALSE, which is not considered a failure. This (apparently) confuses the code in PeekOffset(), which keeps trying forever. I'm not exactly sure about the correctness of this patch, as the convoluted logic here is beyond my grasp right now. Even if we take this fix, I still think we should also take the one I suggested earlier.
Comment on attachment 341742 [details] [diff] [review] patch Can we have a mochitest for this?
Attachment #341742 -
Flags: superreview+
Attachment #341742 -
Flags: review?(roc)
Attachment #341742 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite?
Assignee | ||
Updated•16 years ago
|
OS: Windows Vista → All
Hardware: PC → All
Version: 1.9.0 Branch → Trunk
Assignee | ||
Comment 9•16 years ago
|
||
Fixed by http://hg.mozilla.org/mozilla-central/rev/729436f1f0d3. I'll work on a mochitest (I might need some help).
Assignee | ||
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Comment 10•16 years ago
|
||
This mochitest hangs in my debug build, because it doesn't contain the patch in this bug yet. Because my build doesn't contain the patch yet, I'm not really sure if the ok() tests are ok. But it seems to me they should pass with this patch. So if you could check that, Uri, that would be nice.
Assignee | ||
Comment 11•16 years ago
|
||
Comment on attachment 342413 [details] [diff] [review] mochitest Thanks, Martijn! You're missing a "[0]" at the end of line 39, but once I fixed that, the test passed for me. Also, please remember to change the bug number everywhere from 448860 to 449076.
Assignee | ||
Comment 12•16 years ago
|
||
BTW, why do you need the setTimeout() here (as opposed to calling focusarea() directly)?
Comment 13•16 years ago
|
||
I didn't really need the setTimeout, it's just something that I kept from the original. I didn't really bother checking if it could be removed. If it can be removed, that's fine.
Assignee | ||
Comment 14•16 years ago
|
||
The test still passes for me when it's removed (i.e., I do "addLoadEvent(focusarea);").
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•