638293 - When the value of <input type=number> changes, update its anonymous text input field child as appropriate

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Comment 1

[Mounir Lamouri (:mounir)]

One of the issues with the frame is the value owning. Ehsan, who work an nsTextEditorState which manage the value owning for <input type=text> and textarea recommends to have the value owned by the content.

See the following log:
[01:24] <volkmar> actually, i'm going to work on <input type=number> frame and i will have to manage the value owning, somewhat
[01:24] <volkmar> i was looking at what was done in nsTextEditorState but it's much more than just the value owning
[01:25] <ehsan-sheriff> can't you do that like what we do for nsFileControlFrame?
[01:25] <volkmar> i thought of that
[01:26] <volkmar> but i wonder how nsFileControlFrame manage default values
[01:26] <volkmar> at least value without a frame
[01:26] <ehsan-sheriff> what do you mean?
[01:27] <ehsan-sheriff> file inputs can't have default values iirc
[01:27] <volkmar> yes, but <input type=number> can
[01:27] <ehsan-sheriff> and?
[01:27] <volkmar> but actually, i'm not sure what i've just said apply
[01:27] <volkmar> <input type=file> value is always owned by the content, right?
[01:29] <ehsan-sheriff> yes
[01:29] <ehsan-sheriff> note that nsFileControlFrame uses an <input type=text> as an anonymous content node
[01:29] <ehsan-sheriff> that's what I'm suggesting that you should do
[01:29] <volkmar> yes but you can't write in it directly
[01:30] <volkmar> actually, i was thinking of making <input type=number> value owned by the anonymous <input type=text> inside of it
[01:30] <volkmar> but i wonder what would be the pros and the cons
[01:31] <ehsan-sheriff> hmm
[01:31] <ehsan-sheriff> that's tricky
[01:31] <ehsan-sheriff> it kind of depends on where you want to do your validations
[01:32] <volkmar> why is that changing anything?
[01:32] <ehsan-sheriff> I think the sanest approach is to make <input type=number> own the value
[01:32] <ehsan-sheriff> and just use the internal <input type=text> to dispaly it
[01:33] <ehsan-sheriff> otherwise you should forward the validations to the internal <input type=text>
[01:33] <ehsan-sheriff> which would make a mess I think

Comment 2

[(no longer active)]

To be more precise, nsTextEditorState's main job is owning the editor, not the value.  The value used to be owned by the frame some of the times only because the editor used to be associated with the frame, and as a bonus of the work in bug 534785, the ownership of the value ended up always on the content node too.

Comment 3

[Mounir Lamouri (:mounir)]

Attachment: Part 1 - Show the value when updated from the content

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 540717 [details] [diff] [review]
Part 1 - Show the value when updated from the content

r=dbaron, though I wonder if the code in nsHTMLInputElement could be called in fewer cases (more specific to <input type=number>)?  Or was it your intent to call it in lots of cases?  If so, maybe add a comment.


And sorry for the massive delay here.

Comment 5

[Mounir Lamouri (:mounir)]

jwatt is going to take this.

Comment 6

[Jonathan Watt [:jwatt]]

This patch doesn't work under certain circumstances that I've been having a little trouble figuring out. A bunch of our tests fail, and in some cases setting the value of an <input type=number> simply appends to its existing value.

Comment 7

[Jonathan Watt [:jwatt]]

Turns out this has nothing to do with the editor getting confused, and everything to do with the fact that this patch just sets the default value of the anonymous text input field. If the value of the text input field has changed, then its default value is ignored.

Comment 8

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 10

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

Comment on attachment 817813 [details] [diff] [review]
patch

Does this all handle reset in case primary frame is created after the initial value of the type="number" has changed?
I don't see a testcase for that. Please add such.

>+  nsCOMPtr<nsIDOMHTMLInputElement> numberContent = do_QueryInterface(mContent);
HTMLInputElement* numberContent = HTMLInputElement::FromContent(mContent);

Comment 11

[Jonathan Watt [:jwatt]]

Comment on attachment 817813 [details] [diff] [review]
patch

(In reply to Olli Pettay [:smaug] from comment #10)
> Does this all handle reset in case primary frame is created after the
> initial value of the type="number" has changed?

Yes, the nsNumberControlFrame::CreateAnonymousContent handles that.

> I don't see a testcase for that. Please add such.

The test for that is the line containing |numbers[2].style.display = 'inline-block'| in show-value.html.

Comment 12

[Jonathan Watt [:jwatt]]

Attachment: patch

Actually that test should be listening for MozReftestInvalidate, not load. Fixing that here, plus the HTMLInputElement::FromContent changes you requested.

Comment 13

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/582687f6eaba

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/582687f6eaba