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 all 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.
I guess this would be slightly cleaner as
(boolean or unrestricted double or DOMString)?
, i.e. move the?
outside. It shouldn't have any normative impact.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 tried making this change to the IDL in chromium, but it actually ended up removing the null/undefined type from the type union... do you think the IDL really should be the same either way? Is there something wrong in chrome's IDL implementation?
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.
Yeah, I guess that is a bug in Chromium! Nullable unions are definitely supported in IDL... we should change the spec, IMO, and file a low-priority Chromium bug. In the meantime we can keep using the
DOMString?
version in the implementation since it won't be observably different.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.
crbug filed: https://bugs.chromium.org/p/chromium/issues/detail?id=1307051
And I just pushed a commit to this PR which moves the question mark outside the parenthesis
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 change means that
<embed hidden=until-found>
will display the whole embed, i.e. it will not hide it. Is that intentional?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 changed this to make hidden=until-found not apply to embed so that embed can continue to have its funny hidden-attribute-ignoring behavior. This aligns with how it is implemented in chromium.
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.
embed doesn't ignore the hidden attribute. It displays inline but with zero width/height.
Before this change,
<embed hidden=until-found>...</embed>
will have 0x0 width/height. After this change,<embed hidden=until-found>...</embed>
will have 300x150 width/height.This is probably not a serious compat issue, since hopefully nobody is using
hidden=until-found
yet. But it doesn't seem like a good behavior either; if you're trying to hide the embed element, and the browser just ignores your request and leaves at its original size, that feels wrong.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 changed the selectors to
[hidden=until-found]:not(embed)
andembed[hidden]
which should prevent changes to embed and make<embed hidden=until-found>
0x0. Does it look good?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 looks good! We should be sure this is tested. I'd also appreciate if @zcorpan had time to weigh in since I believe he was discussing hidden embeds earlier in the PR, but we don't need to block on that if he's busy.
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 makes sense. Maybe
until-found
should be disallowed onembed
(as a document conformance requirement)?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.
Although that makes sense, I think it's probably not worth it, as I'd rather just work to deprecate embed (and maybe object) in general.