Last Comment Bug 657861 - HTML5 Textarea with a placeholder and focus and non-focus styles causes FF to consume 100% cpu when cursor is moved out of textarea.
: HTML5 Textarea with a placeholder and focus and non-focus styles causes FF to...
Status: VERIFIED FIXED
[fixed by bug 598833]
: hang, regression, testcase
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla7
Assigned To: Mounir Lamouri (:mounir)
:
: Jet Villegas (:jet)
Mentors:
Depends on: 598833
Blocks: 457801
  Show dependency treegraph
 
Reported: 2011-05-17 20:55 PDT by Greg Clarke
Modified: 2013-12-27 14:22 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
affected
affected
fixed


Attachments
Minimal testcase. (325 bytes, text/html)
2011-05-17 20:57 PDT, Greg Clarke
no flags Details
Testcase with <input> and <textarea> (767 bytes, text/html)
2011-05-18 03:44 PDT, Mounir Lamouri (:mounir)
no flags Details
Testcase with <input> and <textarea> (835 bytes, text/html)
2011-05-27 06:16 PDT, Mounir Lamouri (:mounir)
no flags Details

Description Greg Clarke 2011-05-17 20:55:14 PDT
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

HTML5 document had a form with a textarea.
The textarea has a placeholder attribute.
CSS styles apply to the textarea when it is in focus and when it is not in focus.

Placing cursor in textarea, typing a few characters, then moving cursor anywhere on html page outside of textarea causes FF to spiral out of control.

You can work-around the problem by removing either of the styles or the placeholder attribute.

Reproducible: Always

Steps to Reproduce:
1. load the html file I've supplied
2. lay cursor down in textarea, type a few characters
3. lay cursor down on html page outside textarea.
4. watch cpu rapidly climb to 100%
5. force kill FF.

Actual Results:  
FF goes out of control - consumes 100% cpu, is not responsive, must be killed.

Expected Results:  
Focus should move out of the textarea.

It was a critical but for me - I couldn't release my application on FF4 until I'd figured out what was happening.  But there is a relatively simple work-around, so I'll put it at Normal.

I will attach the testcase file.
Comment 1 Greg Clarke 2011-05-17 20:57:03 PDT
Created attachment 533166 [details]
Minimal testcase.
Comment 2 Alice0775 White 2011-05-17 22:18:38 PDT
Regression range(cached m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/323eb3b2eb2a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100824 Minefield/4.0b5pre ID:20100824230530
Hang;
http://hg.mozilla.org/mozilla-central/rev/a758e294ed9e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100824 Minefield/4.0b5pre ID:20100825003526
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=323eb3b2eb2a&tochange=a758e294ed9e
Comment 3 Timothy Nikkel (:tnikkel) 2011-05-17 22:32:22 PDT
Could be one of Mounir's changes.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-05-17 22:39:19 PDT
I'd assume this is a regression from bug 457801.

Mounir, that testcase will reframe the input when you unfocus.  Does that matter?  Not sure why the transition would matter....
Comment 5 Thomas Ahlblom 2011-05-17 23:23:14 PDT
Reproduced.
Mozilla/5.0 (X11; Linux x86_64; rv:6.0a1) Gecko/20110517 Firefox/6.0a1
Comment 6 Mounir Lamouri (:mounir) 2011-05-18 03:30:46 PDT
It looks very similar to bug 598726.
Comment 7 Mounir Lamouri (:mounir) 2011-05-18 03:40:12 PDT
It seems that bug 598726 has been only partially fixed: the bug doesn't appear anymore for <input> elements but it appears for <textarea>. If you try the testcase in bug 598726 with a <textarea>, you will experience the same issue.
Comment 8 Mounir Lamouri (:mounir) 2011-05-18 03:44:36 PDT
Created attachment 533231 [details]
Testcase with <input> and <textarea>
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2011-05-18 09:59:51 PDT
So do we actually end up hanging, or are we responsive but taking up 100% CPU?
Comment 10 Asa Dotzler [:asa] 2011-05-19 00:13:50 PDT
I get a full browser hang with today's m-c build on Windows 7.
Comment 11 Asa Dotzler [:asa] 2011-05-19 00:15:52 PDT
How common is this likely to be out on the Web? Are many big sites putting placeholder text into <textareas>? Can we chase a fix for Firefox 6 and not sweat it for Firefox 5?
Comment 12 Mounir Lamouri (:mounir) 2011-05-26 11:09:20 PDT
It looks like with end up in an infinite loop calling:
1. PresShell::FlushPendingNotifications(mozFlushType)
2. PresShell::ProcessReflowCommands(int)
3. PresShell::DidDoReflow(int)
4. PresShell::HandlePostedReflowCallbacks(int)
-> back to 1.
Comment 13 Timothy Nikkel (:tnikkel) 2011-05-26 13:39:20 PDT
So I guess it's the nsGfxScrollFrameInner::ReflowFinished reflow callback that keeps getting posted and keeps return true so we keep flushing?
Comment 14 :Ehsan Akhgari 2011-05-26 13:43:39 PDT
(In reply to comment #11)
> How common is this likely to be out on the Web? Are many big sites putting
> placeholder text into <textareas>? Can we chase a fix for Firefox 6 and not
> sweat it for Firefox 5?

I believe this is possible to hit on real web content, but not very likely.  Note that it seems that bug 598726 was originally filed by a web developer.
Comment 15 :Ehsan Akhgari 2011-05-26 13:46:02 PDT
It is worth for somebody to see if something like https://bugzilla.mozilla.org/show_bug.cgi?id=598726#c13 is happening here.  Step 6 in that list might change since we should have initialized the editor when the control's frame was created...
Comment 16 Timothy Nikkel (:tnikkel) 2011-05-26 14:23:03 PDT
Oddly the testcase doesn't hang for me on Linux.
Comment 17 Mounir Lamouri (:mounir) 2011-05-27 06:16:53 PDT
Created attachment 535615 [details]
Testcase with <input> and <textarea>

My bad, the testcase wasn't correct: you have to write something in the text fields.
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2011-06-06 15:34:45 PDT
Mounir, can you look into this or find a more appropriate owner if you're not it? Given that we shipped 4 with this and it's not something that's been seen a lot in the wild we won't hold 5 for this.
Comment 19 Mounir Lamouri (:mounir) 2011-06-07 02:43:33 PDT
This bug seems to have been fixed. Something that sounds possible is that Ehsan's hint was correct (comment 15) and bug 598833 fixed the issue given that we no longer notify changes when there are no changes.

Though, I have this assert:
###!!! ASSERTION: Unexpected document: 'capturingContent->GetCurrentDoc() == GetDocument()', file /Users/volkmar/projects/mozilla/html5forms/layout/base/nsPresShell.cpp, line 6694

I will do some investigation to be check why and when the bug has been fixed.
Comment 20 Mounir Lamouri (:mounir) 2011-06-07 02:54:17 PDT
It has been fixed in this range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c6d107ede5a&tochange=9a6c139a4e58

Which includes bug 598833, as expected.
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2011-06-07 08:49:13 PDT
Hrm.  But notifying changes when there have been no changes was supposed to be a no-op all along, no?  Unless one of the bits that always cause a reframe was notified...
Comment 22 Mounir Lamouri (:mounir) 2011-06-07 10:17:32 PDT
(In reply to comment #21)
> Hrm.  But notifying changes when there have been no changes was supposed to
> be a no-op all along, no?  Unless one of the bits that always cause a
> reframe was notified...

I probably didn't understood what you meant but in nsCSSFrameConstructor::ContentStateChanged, |PostRestyleEvent| is called and there is no check for a real change before that so it's probably not a no-op.

So, to summary, when OnValueChanged was called, if there was a placeholder, a document and the element was focused, a state change was notified without checking if that was needed because we didn't know what was the previous value [1]. Now, we could optimize that a bit by checking mStates | NS_EVENT_STATES_MOZ_PLACEHOLDER and check if that match the current placeholder style. Though, that's probably not critical and will only prevent one call to IntrinsicState().

[1] Basically that should be done if value becomes the empty string or is no longer the empty string, which requires to know what value _was_.
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2011-06-07 10:21:42 PDT
PostRestyleEvent is called, but the actual restyle should be a no-op if nothing has actually changed, right?  Was that not happening for some reason?
Comment 24 Mounir Lamouri (:mounir) 2011-06-07 10:32:00 PDT
(In reply to comment #23)
> PostRestyleEvent is called, but the actual restyle should be a no-op if
> nothing has actually changed, right?  Was that not happening for some reason?

Could that be because it's part of an animation?
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2011-06-07 10:48:54 PDT
Oh, yes.  We might be getting random reframing there...  seems odd, if so, but not entirely impossible.  May be worth a followup bug, though now it's hard to reproduce.
Comment 26 Virgil Dicu [:virgil] [QA] 2011-08-19 04:26:56 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0

Verified with the test case from comment 17.

The issue is no longer reproducible. The browser no longer hangs after following the steps provided there.

Setting status to verified fixed.

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