Closed
Bug 586662
Opened 14 years ago
Closed 14 years ago
Window locks up when textarea keypress event handler sets overflow style to hidden
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b7
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: cplarosa, Assigned: ehsan.akhgari)
References
Details
(Keywords: regression, testcase)
Attachments
(4 files, 2 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3) Gecko/20100805 Firefox/4.0b3
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3) Gecko/20100805 Firefox/4.0b3
Browser window locks up when textarea overflow style is set to hidden in a keypress event handler.
Reproducible: Always
Steps to Reproduce:
1. Create a page with the following HTML code:
<TEXTAREA onkeypress="this.style.overflow = 'hidden'"></TEXTAREA>
2. Open the web page, click in the text box, and type a single character
3. The cursor stops blinking, and the character is not entered in the box
Actual Results:
The cursor stops blinking and the browser page stops accepting input. The page will also not repaint if the browser in minimized and restored, so it appears that the window has locked up. If the page contains multiple input fields, you cannot select or enter data into any of them. The only thing you can do is close the window.
Expected Results:
The window continues to accept input and behave normally.
This problem occurs in Firefox 4 beta 1, 2, and 3. It does not occur in Firefox 3.x or other browsers.
Updated•14 years ago
|
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
Comment 2•14 years ago
|
||
Comment 3•14 years ago
|
||
Seems plausible, yes. Focus goes into lala-land, perhaps?
Blocks: 534785
blocking2.0: --- → ?
Comment 4•14 years ago
|
||
I have a feeling this is a similar issue to bug 592663.
Comment 5•14 years ago
|
||
I get these assertions, similar to bug 592663. The unbalanced update view batch count would explain most of the problems here.
###!!! ASSERTION: bad action nesting!: 'mActionNesting>0', file ../../../../editor/libeditor/text/nsTextEditRules.cpp, line 250
###!!! ASSERTION: bad state: 'mUpdateCount > 0', file ../../../../editor/libeditor/base/nsEditor.cpp, line 4080
###!!! ASSERTION: Someone forgot to call EndUpdateViewBatch!: '!mRootVM', file ../../dist/include/nsIViewManager.h, line 297
Comment 6•14 years ago
|
||
I don't get the last assertion on Mac, but definitely looks like painting/updating just stops happening. Focus and other behaviour is still happening properly, but there just isn't any visible effect.
blocking2.0: ? → betaN+
Assignee: nobody → ehsan
Comment 7•14 years ago
|
||
Bug 240933 would probably fix most of the problems here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 8•14 years ago
|
||
(In reply to comment #7)
> Bug 240933 would probably fix most of the problems here.
You're right. I got following Progression Range on MC where the Issue is fixed.
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=886665dec3cb&tochange=f2f217290bd0
So when Bug 240933 relands this should be okay. Does this make Bug 240933 a Blocker now? :)
Depends on: 240933
Assignee | ||
Comment 9•14 years ago
|
||
Bug 240933 fixes this in the sense that the browser doesn't lock up, but still the first character typed is lost.
And I get a lot of "ASSERTION: Bad mBatching: 'mBatching >=0'" assertions.
Assignee | ||
Comment 10•14 years ago
|
||
So, the reason this happens is that we flush here: <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/base/nsEditor.cpp#4209>, which causes the textarea to be reframed, which will reset the selection object, initializing mBatching to 0, and the next EndBatchChanges call will try to decrement it to -1.
This flush is coming from bug 393723. Boris, do you think that this is still necessary?
A dirty hack which might work here is to put a script blocker in nsPlaintextEditor::TypedText...
Comment 11•14 years ago
|
||
Or to always do the scroll async in ScrollSelectionIntoView, right?
I think that as long as we keep doing sync scrolls we need the flush there, though you could try taking it out and seeing whether bug 393723 reappears.
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11)
> I think that as long as we keep doing sync scrolls we need the flush there,
> though you could try taking it out and seeing whether bug 393723 reappears.
I tried it, and it seems that backing out bug 393723 doesn't cause the problem to reappear. I added a test case for that bug to make sure that we don't regress this in the future.
With this patch, I don't get any assertions, but still the first character typed into the field gets eaten up. I'm still investigating that.
Attachment #477323 -
Flags: review?(roc)
Assignee | ||
Comment 13•14 years ago
|
||
In fact, it seems that we can always do an async scroll. This patch fixes the key entry problem, and with this, the bug can be considered fully fixed.
Attachment #477361 -
Flags: review?(roc)
Assignee | ||
Comment 14•14 years ago
|
||
These patches caused the assertion count in <http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/crashtests/448329-3.html?force=1> to jump up on Mac. So, I thought to go ahead and fix the assertions instead of just increasing them in the manifest.
Attachment #477611 -
Flags: review?(roc)
Assignee | ||
Comment 15•14 years ago
|
||
This also depends on bug 174823, because if we refresh immediately, we might end up flushing as well, which is what happens on Windows and Linux, according to the try server.
I'll post a patch to that bug shortly.
Assignee | ||
Comment 16•14 years ago
|
||
With one other assertion count adjusted.
Attachment #477611 -
Attachment is obsolete: true
Attachment #477818 -
Flags: review?(roc)
Attachment #477611 -
Flags: review?(roc)
Attachment #477323 -
Flags: review?(roc) → review+
Attachment #477361 -
Flags: review?(roc) → review+
Comment on attachment 477818 [details] [diff] [review]
Part 3: Don't attempt to compare unrelated points
+ if (!aCompareNode->IsInDoc() || !end->IsInDoc() ||
+ aCompareNode->GetOwnerDoc() != end->GetOwnerDoc()) {
Just check aCompareNode->GetCurrentDoc() != end->GetCurrentDoc() || !end->GetCurrentDoc().
Attachment #477818 -
Flags: review?(roc) → review+
Assignee | ||
Comment 18•14 years ago
|
||
With comments addressed.
Attachment #477818 -
Attachment is obsolete: true
Assignee | ||
Updated•14 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [needs landing]
Assignee | ||
Comment 20•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b138e7c0fdf2
http://hg.mozilla.org/mozilla-central/rev/eb27a2b76c40
http://hg.mozilla.org/mozilla-central/rev/ed8fed2ca7af
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Updated•14 years ago
|
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in
before you can comment on or make changes to this bug.
Description
•