259636 - Should setting .value on text inputs flush reflows?

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

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 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Perhaps like this

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

Can anyone see a reason this would be a bad idea?

Comment 3

[kinmoz]

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?

Comment 4

[Kathleen :Brade]

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

temporary r- until there is a response to Kin's comments

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase for comment 3 paragraph 2

Comment 6

[Boris Zbarsky [:bzbarsky]]

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.

Comment 7

[Boris Zbarsky [:bzbarsky]]

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

Kin, with bug 244366 fixed the various testcases no longer flicker with this
patch....

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Attachment: reverse textarea

Works well with the patch.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Sounds good.  If things look good in your testing, let's get this landed!

Comment 12

[Boris Zbarsky [:bzbarsky]]

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

Since smaug says this behaves well...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/678e3c57654a