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.

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Greg Clarke]

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]

Attachment: Minimal testcase.

Comment 2

[Alice0775 White]

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)]

Could be one of Mounir's changes.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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]

Reproduced.
Mozilla/5.0 (X11; Linux x86_64; rv:6.0a1) Gecko/20110517 Firefox/6.0a1

Comment 6

[Mounir Lamouri (:mounir)]

It looks very similar to bug 598726.

Comment 7

[Mounir Lamouri (:mounir)]

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)]

Attachment: Testcase with <input> and <textarea>

Comment 9

[Boris Zbarsky [:bzbarsky]]

So do we actually end up hanging, or are we responsive but taking up 100% CPU?

Comment 10

[Asa Dotzler [:asa]]

I get a full browser hang with today's m-c build on Windows 7.

Comment 11

[Asa Dotzler [:asa]]

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)]

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)]

So I guess it's the nsGfxScrollFrameInner::ReflowFinished reflow callback that keeps getting posted and keeps return true so we keep flushing?

Comment 14

[(no longer active)]

(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

[(no longer active)]

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)]

Oddly the testcase doesn't hang for me on Linux.

Comment 17

[Mounir Lamouri (:mounir)]

Attachment: 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)]

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)]

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)]

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 [:bzbarsky]]

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)]

(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 [:bzbarsky]]

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)]

(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 [:bzbarsky]]

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]]

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.