Should setting .value on text inputs flush reflows?

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
Layout: Form Controls
P2
normal
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

(Blocks: 1 bug, {perf})

Trunk
mozilla1.9.3a1
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Right now, setting .value on a text input sets the value and flushes reflows
(instead of just posting a reflow event).

Editor does this so that things updated immediately when typing.  But for
setting .value, I don't think that's desirable.
Comment on attachment 159010 [details] [diff] [review]
Perhaps like this

Can anyone see a reason this would be a bad idea?
Attachment #159010 - Flags: superreview?(dbaron)
Attachment #159010 - Flags: review?(brade)

Comment 3

14 years ago
Hey bz, as I mentioned on IRC, checkout bug 174823 and bug 141900 for some
background on the problems with async reflow.

How does your patch act on a page where there's an event handler (filter) that
also sets the value of the text widget?
Attachment #159010 - Flags: superreview?(dbaron) → superreview+

Comment 4

14 years ago
Comment on attachment 159010 [details] [diff] [review]
Perhaps like this

temporary r- until there is a response to Kin's comments
Attachment #159010 - Flags: review?(brade) → review-
Created attachment 159752 [details]
Testcase for comment 3 paragraph 2
The testcase seems to work OK over here in a patched build on Linux, but maybe I
just type too slowly...

My build _does_ show bug 165130, and I would fully expect that bug 158782 and
bug 151882 would show up when the filter is being used (though I can't really
reproduce them, as I said).

It sounds like we should make bug 174823 somewhat more of a priority...  Should
this one be marked dup of it?  This is a more modest proposal, but it seems like
it has all the same issues.
Depends on: 174823
Attachment #159010 - Flags: review- → review?(kinmoz)
Comment on attachment 159010 [details] [diff] [review]
Perhaps like this

Kin, with bug 244366 fixed the various testcases no longer flicker with this
patch....
Priority: -- → P2
Summary: Should setting .value on text inputs flush reflows? → [FIX]Should setting .value on text inputs flush reflows?
Target Milestone: --- → mozilla1.8beta
Comment on attachment 159010 [details] [diff] [review]
Perhaps like this

Talked to kin some more.  There are also caret-painting issues, and since that
does not go through the viewmanager those are not resolved yet... (to be
precise, deleting content leaves the caret no place to go so it flickers to
beginning of line).
Attachment #159010 - Flags: review?(kinmoz) → review-
Summary: [FIX]Should setting .value on text inputs flush reflows? → Should setting .value on text inputs flush reflows?
The patch for Bug 518122 does the same what this does.
When using the testcase in this bug, the patch for bug 518122 and bug 518950
fixes the flickering I otherwise see on trunk builds. (At least the spellcheck markers flickers.)

But I need to test something else; perhaps something like reversing the user input
in keypress handler. That would prevent the value cache the patch for bug 518122
adds.
Blocks: 518122
Created attachment 403209 [details]
reverse textarea

Works well with the patch.
Sounds good.  If things look good in your testing, let's get this landed!
Comment on attachment 159010 [details] [diff] [review]
Perhaps like this

Since smaug says this behaves well...
Attachment #159010 - Flags: review- → review?(roc)
Pushed http://hg.mozilla.org/mozilla-central/rev/678e3c57654a
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.8beta1 → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.