Editorial updates to attribute reflection and enumerated attributes #7956
Conversation
This is easier to read, partially because it spreads out the steps and branches, and partially because of the asserts making some conditions explicit.
Notably this gives an explicit algorithm for determining the state, instead of a couple paragraphs of prose. It also broadens the definition of "canonical keyword" to automatically be determined in simpler cases, instead of only used in tricky cases.
| <ol> | ||
| <li><p>If the content attribute is not in any state (e.g. the attribute is missing and there | ||
| is no <i data-x="missing value default">missing value default</i>), or the content attribute | ||
| is in a state with no associated keyword value, then return the empty string.</p></li> |
There was a problem hiding this comment.
Use "no state" here for consistency with the other algorithm?
What is a state with no associated keyword value? Is that "no state" due to lack of an invalid value default?
There was a problem hiding this comment.
I wonder if we should get rid of the "no state" state, and require invalid value default and missing value default for all enumerated attributes. I think it would make things simpler and less confusing.
There was a problem hiding this comment.
What is a state with no associated keyword value? Is that "no state" due to lack of an invalid value default?
No, it's something like the inherit state for contenteditable
I wonder if we should get rid of the "no state" state, and require invalid value default and missing value default for all enumerated attributes. I think it would make things simpler and less confusing.
I was thinking of that. It would amount to introducing a lot of individual "default" states for a few attributes. In particular for: dir="", <link as="">, <meta http-equiv="">, formmethod=""/formenctype="", inputmode="", and enterkeyhint="". It's probably a clarity improvement but it's also a good bit of work.
There was a problem hiding this comment.
OK. Considering the risk of introducing bugs, maybe it's not worth it, and it's sufficiently clear with this PR?
There was a problem hiding this comment.
contenteditable isn't limited to only known values though. I'm not entirely convinced we ever hit this branch looking through the callers, but we could leave it as follow-up.
There was a problem hiding this comment.
You're right about contenteditable. I think dir="" hits this branch though. It is limited to only known values, and has no invalid value default or missing value default, so if it's just omitted then it will end up in the "not in any state" case, and return the empty string. http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=10361
|
Also update |
|
Actually maybe we should have "limited to only known values" exist for both nullable and non-nullable enumerated attributes. I think that would be clearer for readers. |
| <ol> | ||
| <li><p>If the content attribute is not in any state (e.g. the attribute is missing and there | ||
| is no <i data-x="missing value default">missing value default</i>), or the content attribute | ||
| is in a state with no associated keyword value, then return the empty string.</p></li> |
There was a problem hiding this comment.
contenteditable isn't limited to only known values though. I'm not entirely convinced we ever hit this branch looking through the callers, but we could leave it as follow-up.
I think this is a good idea. Still to-do before we can merge this; I ran out of time for today. |
|
OK, this is ready for re-review. I realized that "limited to only known values" applies to IDL attributes, not content attributes. Thus, it should be stated where the I also changed the |
Myself and @zcorpan got ourselves confused around openui/open-ui#533 (comment) . So I rewrote the confusing sections.
/common-dom-interfaces.html ( diff )
/common-microsyntaxes.html ( diff )
/urls-and-fetching.html ( diff )