Closed
Bug 605741
Opened 14 years ago
Closed 14 years ago
entire textarea gets invalidated with every character typed
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jst
:
review+
mounir
:
feedback+
dbaron
:
approval2.0+
|
Details | Diff | Splinter Review |
nsHTMLTextAreaElement::OnValueChanged sends ContentStatesChanged whenever it is called. Because textareas usually have a native appearance set this causes nsCSSFrameConstructor::DoContentStateChanged to add the repaint frame hint. We should be smarter and only send this notifications when we need to.
Assignee | ||
Comment 1•14 years ago
|
||
Assignee: nobody → tnikkel
Attachment #484632 -
Flags: review?(jst)
Comment 2•14 years ago
|
||
Comment on attachment 484632 [details] [diff] [review]
patch
>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>--- a/content/html/content/src/nsHTMLInputElement.cpp
>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>@@ -3850,22 +3850,24 @@ void
> void
> nsHTMLInputElement::UpdateAllValidityStates(PRBool aNotify)
> {
>+ PRBool validBefore = IsValid();
> UpdateTooLongValidityState();
> UpdateValueMissingValidityState();
> UpdateTypeMismatchValidityState();
> UpdatePatternMismatchValidityState();
>-
>- if (aNotify) {
>+ PRBool validAfter = IsValid();
>+
>+ if (aNotify && validBefore != validAfter) {
I think |if (validBefore != IsValid() && aNotify)| might be better. No need to save the return value of IsValid() in validAfter and aNotify might be true very often (false mostly while parsing).
>diff --git a/content/html/content/src/nsHTMLTextAreaElement.cpp b/content/html/content/src/nsHTMLTextAreaElement.cpp
>--- a/content/html/content/src/nsHTMLTextAreaElement.cpp
>+++ b/content/html/content/src/nsHTMLTextAreaElement.cpp
> NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::OnValueChanged(PRBool aNotify)
> {
> // Update the validity state
>+ PRBool validBefore = IsValid();
> UpdateTooLongValidityState();
> UpdateValueMissingValidityState();
>+ PRBool validAfter = IsValid();
>
[...]
>+ PRInt32 stateMask = 0;
>+ if (validBefore != validAfter) {
validBefore != IsValid() ?
Attachment #484632 -
Flags: feedback+
Updated•14 years ago
|
Attachment #484632 -
Flags: review?(jst) → review+
Assignee | ||
Updated•14 years ago
|
Attachment #484632 -
Flags: approval2.0?
Assignee | ||
Comment 3•14 years ago
|
||
Thanks for the comments. I made those changes in my queue.
Updated•14 years ago
|
Attachment #484632 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Comment 4•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•14 years ago
|
||
Mounir, are there other cases where we send ContentStatesChanged notifications when we don't need to? In addition to skipping a lot of unneeded painting fixing this bug also sped up bug 473178, so there might be some good wins to be had.
You need to log in
before you can comment on or make changes to this bug.
Description
•