Closed Bug 997805 Opened 6 years ago Closed 6 years ago

Placeholder not showing in textarea after it is hidden and then re-shown.

Categories

(Core :: Layout: Form Controls, defect)

17 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: quazzieclodo, Assigned: ehsan)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20131029 Firefox/17.0 (Nightly/Aurora)
Build ID: 20131029172439

Steps to reproduce:

Create a textarea with a placeholder tag.
Hide the element with javascript and set its value to some text.
Show the element and set its value to "".

jsfiddle (click "Hide" and then "Show"): http://jsfiddle.net/65rpe/3/


Actual results:

The placeholder does not appear in the textarea.
I have to enter some text and then delete it for the placeholder to reappear.


Expected results:

The placeholder should have reappeared in the textarea once it was shown and its value was set to "".
I reproduce on fx29. may related to bug 673873.
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Core & HTML
Ever confirmed: true
Product: Firefox → Core
Component: DOM: Core & HTML → Layout: Form Controls
Recomputing the placeholder visibility does not require the placeholder div
itself to be present, as the only information required for that is the current
value of the text control which is present either way.  This patch fixes
nsTextEditorState::ValueWasChanged and nsTextEditorState::UpdatePlaceholderVisibility
to that effect.

But the real fix is in nsTextEditorState::UpdatePlaceholderText, where after
setting the placeholder text on the anonymous div, we redo the placeholder
visibility computation.  Since this function can be called from
HTMLTextAreaElement::CreatePlaceholderNode during frame construction, the
GetValue function may return the wrong value since the editor has not properly
been set up yet, resulting in this bug.  And this function call is useless
anyway, because changing the placeholder text does not really affect the
result of the visibility computation, so there is no need to do this work
in the first place.
Attachment #8409492 - Flags: review?(bzbarsky)
https://tbpl.mozilla.org/?tree=Try&rev=ac4ff8214264
Assignee: nobody → ehsan
Comment on attachment 8409492 [details] [diff] [review]
Correctly restore the placeholder text after the editor object is re-attached to a text control as a result of a reframe; r=bzbarsky

r=me
Attachment #8409492 - Flags: review?(bzbarsky) → review+
(In reply to Wes Kocher (:KWierso) from comment #6)
> I had to back this out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7a633ff64906 for
> breaking reftests:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=38519180&tree=Mozilla-
> Inbound
> https://tbpl.mozilla.org/php/getParsedLog.php?id=38519086&tree=Mozilla-
> Inbound

That is really weird!  This is the test that I landed here, which did pass through try in comment 3, and I double checked to make sure the test actually ran...  Just checked again and the test does pass locally.  Any idea what I'm doing wrong? :/
Flags: needinfo?(ryanvm)
Flags: needinfo?(kwierso)
Flags: needinfo?(ehsan)
Ah, I can reproduce locally on the inbound tip...  So this has bitrotten :(
Flags: needinfo?(ryanvm)
Flags: needinfo?(kwierso)
Haha, this is broken because of the patch in bug 821307!  I guess I get what I deserve for reviewing patches fast! ;)
Blocks: 821307
(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #9)
> Haha, this is broken because of the patch in bug 821307!  I guess I get what
> I deserve for reviewing patches fast! ;)

Bug 821307 actually unveils an unrelated bug which just happens to affect this test case.
Blocks: 1001936
No longer blocks: 821307
https://hg.mozilla.org/mozilla-central/rev/b9ed93ef1d23
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Depends on: 1088158
You need to log in before you can comment on or make changes to this bug.