Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add hidden=until-found HTML attribute and beforematch event #7475
Add hidden=until-found HTML attribute and beforematch event #7475
Changes from 29 commits
30b0552
61a3a41
387c131
9d25907
d04e4d6
93e1841
8089440
12cbbda
ca8eb40
0d6b2eb
b411034
7aace23
65448e1
ca6d534
e725820
09fdbde
3dc6833
206a802
f7a1366
08adbe7
16eaebe
08635ea
e8d2b9a
e701e98
90e266c
81c9644
d18bd7b
9321fb7
63060cf
1cb11b2
05c2dbf
ec99d06
8fe8e25
642c996
732f694
ab5485b
0bb2751
be0e978
a065268
064105d
1466e3c
e380a50
a5f07b8
ad370de
bb761d0
95042f1
377a2d5
3b7158c
d043b77
5d51c96
24a2b8f
e0726f9
c955cf3
bfcb672
cb88ab9
39db0dc
ecd4a32
c173bd4
282af51
75621bd
f3456c2
ec6f978
16ceccc
383e976
291e33c
fa64e8c
c9bdf96
c885c5e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to specify that the element is an enumerated attribute, per the comment in #6040 (comment). See other examples of enumerated attribute, and how they specify keywords and states, by searching for "enumerated attribute" in https://html.spec.whatwg.org/ .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem like an enumerated attribute because instead of having a bunch of different relevant string values, it has these relevant states:
Should I still say that it's an enumerated attribute anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enumerated attributes are exactly attributes with multiple states. "not set" = missing valid default state. anything else = invalid value default state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks!
I made it an enumerated attribute with a table like the other enumerated attributes.
How does it look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs a lot more details, about the timing of this event, whether it can be default-prevented, what should the UA do if the element is still hidden after the event fires, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is non-normative. The normative part is at https://github.com/whatwg/html/pull/7475/files#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947dR74841
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That still doesn't mention whether the event should bubble, whether it's composed, etc, right? And that algorithm is only called from find-in-page, but then some other bits mention fragment navigation too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does: "fire an event" is defined here: https://dom.spec.whatwg.org/#concept-event-fire -- the IDL members (e.g.
bubbles
) have default values.The fragment navigation part is here I think: https://github.com/whatwg/html/pull/7475/files#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947dR77882
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should check if the content attribute is set, to match reflecting boolean IDL attribute getter behavior.
https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I made it look at the attribute instead of
<var>value</var>
. How does it look? Is it OK that I didn't use the term "content attribute"?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove "run the following steps"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, the sentence now ends with "then:"