Set dirty value flag for setRangeText() #2435

annevk · 2017-03-14T10:31:13Z

domenic · 2017-03-14T15:13:04Z

Content LGTM. Editorially this is strange; just reading the text it sounds weird because two different concepts are using the term "dirty value flag", so unless you hover over the links, the sentence just looks very wrong. I'm not sure if there's any good fix for that. Maybe "input dirty value flag" and "textarea dirty value flag"?

annevk · 2017-03-14T15:15:57Z

I think the right fix would be to make it a shared concept, but I was lazy. As for tests, the issue mentions some that could be uplifted from Gecko.

domenic · 2017-03-14T21:25:24Z

#setting-minimum-input-length-requirements:-the-minlength-attribute seems to use <var>dirty value flag</var>. That seems a bit better to me?

annevk · 2017-03-15T08:20:01Z

@domenic that seems fine. Do you think it makes sense for two elements to share a slot? I didn't investigate if we already do that for input and textarea, I thought we might. I could file a follow-up to investigate that.

@bzbarsky would you be okay uplifting the tests from Gecko?

bzbarsky · 2017-03-15T14:55:02Z

@bzbarsky would you be okay uplifting the tests from Gecko?

Absolutely. I can just create a wpt PR, or move them to the to-be-synced bits in Gecko's tests and wait for jgraham to uplift them, or whatever. Just let me know.

annevk · 2017-03-15T16:42:53Z

@bzbarsky whatever is most expedient for you is fine. If you could leave some pointer here of that happening that should be sufficient for archeologists.

bzbarsky · 2017-03-15T16:47:22Z

I'll do this gecko-side, then. Minimal merge pain for me. Will land it once this PR is in.

annevk · 2017-03-15T16:49:56Z

Okay, I think it's fine to land it now as long as the commit message includes a pointer here, but up to you. This PR is blocked on some feedback from @domenic.

domenic · 2017-03-15T17:31:59Z

Do you think it makes sense for two elements to share a slot? I didn't investigate if we already do that for input and textarea, I thought we might. I could file a follow-up to investigate that.

I do. Yesterday I thought that there was no good place to put the definition, but now I'm thinking the section "A form control's value" is a pretty good place for it. Maybe I'm forgetting some reason that made less sense yesterday.

We can also do the <var> version for now and file a follow-up issue for the share-a-slot version. Heck, we can do the awkward-phrasing version in this PR if you really want; it's not a big deal, as long as there's a follow-up issue filed.

annevk · 2017-03-15T18:24:54Z

I tried to fix both here. Used distinct commits to separate editorial from normative change.

domenic

I think we need a better description of what it is, or leave the description out. Sorry this is dragging on a bit.

domenic · 2017-03-15T21:34:18Z

source

@@ -49906,7 +49903,7 @@ interface <dfn>HTMLTextAreaElement</dfn> : <span>HTMLElement</span> {

  <p>The <dfn><code data-x="attr-textarea-maxlength">maxlength</code></dfn> attribute is a <span
  data-x="attr-fe-maxlength">form control <code data-x="">maxlength</code> attribute</span> controlled
-  by the <code>textarea</code> element's <span data-x="concept-textarea-dirty">dirty value
+  by the <code>textarea</code> element's <span data-x="concept-fe-dirty">dirty value


Here and below, I think the "controlled by the X element's dirty value flag" is now redundant. That's generically part of the definition of form control maxlength attribute now.

I ended up removing this four times.

domenic · 2017-03-15T21:35:47Z

source

@@ -51118,6 +51115,10 @@ interface <dfn>HTMLLegendElement</dfn> : <span>HTMLElement</span> {
  email fields might translate that into a <span data-x="concept-fe-value">value</span> of "<code
  data-x="">[email protected]</code>" (without the leading whitespace).</p>

+  <p id="concept-input-value-dirty-flag"><span id="concept-textarea-dirty"></span><code>input</code>
+  and <code>textarea</code> elements have a <dfn data-x="concept-fe-dirty">dirty value flag</dfn>.
+  This is used to determine what to display to the user.</p>


I don't think this is an accurate description of how it's used. I would say it's used to determine the interaction between the DOM structure (value content attribute or text node children) and the form control's value. Not sure how to phrase "DOM structure" better.

The simplest thing may be to say that it's used to determine the interaction between the default value and the value. If the flag is not set, the value tracks the default value. If it's set, the value is independent of the default value.

Thanks, I contrasted value and the value content attribute since we don't really have a default value concept per se. I used data-x="" for the value content attribute since I didn't want to reiterate the input/textarea distinction.

I don't think that's great, because now the definition only applies to input elements. The "default value" concept does somewhat exist; the spec says

The contents of the control represent the control's default value.

for textarea and

The value content attribute gives the default value of the input element.

for input.

How does it only apply to input elements?

Oh right, doh.

Fixes #2425.

annevk · 2017-03-16T15:26:20Z

Apologies, pushed a new version that just says default value as it should.

domenic added topic: forms needs tests Moving the issue forward requires someone to write tests labels Mar 14, 2017

domenic removed the needs tests Moving the issue forward requires someone to write tests label Mar 14, 2017

annevk force-pushed the annevk/setrangetext branch from 50d7008 to 56c35cc Compare March 15, 2017 18:24

domenic requested changes Mar 15, 2017

View reviewed changes

annevk force-pushed the annevk/setrangetext branch from 56c35cc to cea1858 Compare March 16, 2017 08:22

annevk added 2 commits March 16, 2017 16:25

Editorial: share the dirty value flag between input and textarea

6ff5baa

Set dirty value flag for setRangeText()

0f20791

Fixes #2425.

annevk force-pushed the annevk/setrangetext branch from cea1858 to 0f20791 Compare March 16, 2017 15:26

domenic approved these changes Mar 16, 2017

View reviewed changes

domenic merged commit 4c404a2 into master Mar 16, 2017

domenic deleted the annevk/setrangetext branch March 16, 2017 15:32

domenic mentioned this pull request Mar 23, 2017

Add img.decode() API for "predecoding" images #2332

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set dirty value flag for setRangeText() #2435

Set dirty value flag for setRangeText() #2435

annevk commented Mar 14, 2017

domenic commented Mar 14, 2017

annevk commented Mar 14, 2017

domenic commented Mar 14, 2017

annevk commented Mar 15, 2017

bzbarsky commented Mar 15, 2017

annevk commented Mar 15, 2017

bzbarsky commented Mar 15, 2017

annevk commented Mar 15, 2017

domenic commented Mar 15, 2017

annevk commented Mar 15, 2017

domenic left a comment

domenic Mar 15, 2017

annevk Mar 16, 2017

domenic Mar 15, 2017

bzbarsky Mar 15, 2017

annevk Mar 16, 2017

domenic Mar 16, 2017

annevk Mar 16, 2017

annevk Mar 16, 2017

annevk Mar 16, 2017

annevk commented Mar 16, 2017

Set dirty value flag for setRangeText() #2435

Set dirty value flag for setRangeText() #2435

Conversation

annevk commented Mar 14, 2017

domenic commented Mar 14, 2017

annevk commented Mar 14, 2017

domenic commented Mar 14, 2017

annevk commented Mar 15, 2017

bzbarsky commented Mar 15, 2017

annevk commented Mar 15, 2017

bzbarsky commented Mar 15, 2017

annevk commented Mar 15, 2017

domenic commented Mar 15, 2017

annevk commented Mar 15, 2017

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Mar 16, 2017