Skip to content
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

New feature proposal: HTMLScriptElement.supports(type) method #6472

Closed
horo-t opened this issue Mar 10, 2021 · 15 comments
Closed

New feature proposal: HTMLScriptElement.supports(type) method #6472

horo-t opened this issue Mar 10, 2021 · 15 comments

Comments

@horo-t
Copy link
Contributor

horo-t commented Mar 10, 2021

Currently there is no simple way to know what kind of types can be used for HTMLScriptElement’s type attribute.

There are several new feature proposals using the script element.

We can use the nomodule attribute to detect the module type support as this example shows.
But there is no unified method to detect such new features. (E.g.: WICG/import-maps#171)
If we have the HTMLScriptElement.supports(type) method which is like HTMLLinkElement.relList.supports(), we can easily detect such features.

The change of the spec will be like this.

@hiroshige-g
Copy link
Contributor

When implementing supports(), should it reflect the actual behavior of <script> on that browser (e.g. returning true for non-standard type attribute values that are allowed in Chromium for legacy reasons)?

@hiroshige-g
Copy link
Contributor

As for the spec change, it's better to split out the Step 8 of #prepare-a-script into a helper method and call it from supports(), to ensure the behavior of these two are the same (also it will further clarify the intention of supports()). In the current commit, they behaves slightly differently due to lack of leading and trailing ASCII whitespace stripped call in supports().

@domenic
Copy link
Member

domenic commented Mar 10, 2021

Another example is https://github.com/jeremyroman/alternate-loading-modes/blob/main/triggers.md#speculation-rules .

FWIW I am cautiously positive on this. Feature-detection interfaces are always a bit tricky, but we have positive experience so far with relList.supports(), and this proposal builds on a shared primitive even more so than relList.supports() does.

Any thoughts from other implementers? @annevk @hober

@annevk
Copy link
Member

annevk commented Mar 11, 2021

What are the non-standard type attribute values? I think it would be cleaner to build the API on top of https://html.spec.whatwg.org/#concept-script-type and only support "classic" and "module".

Limiting the type IDL attribute to known values is probably a breaking change at this point, so this seems reasonable overall.

@horo-t
Copy link
Contributor Author

horo-t commented Mar 19, 2021

It sounds reasonable to me that support() only supports "classic" and "module".
What do you think? @hiroshige-g @domenic

@domenic
Copy link
Member

domenic commented Mar 19, 2021

I'm a bit torn. I like the simplicity of that. But then it's totally disconnected from the type="" attribute.

It does match the { type } option to new Worker() though, so maybe that's good enough precedent.

@hiroshige-g
Copy link
Contributor

Supporting only "classic" and "module" sounds good, while it deviates from type attribute and DOMTokenList approach.

To align with WorkerOptions, we should do exact match, i.e. strings with leading/trailing white spaces and uppercases are not considered supported.
(relList.supports() allows uppercases but not whitespaces, <script type> attribute allows both though, but practically I don't expect this is problematic)

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 2, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 2, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
horo-t added a commit to horo-t/html that referenced this issue Sep 2, 2021
@horo-t
Copy link
Contributor Author

horo-t commented Sep 2, 2021

Thank you. Disallowing uppercases sounds good to me. Updated spec change.

@horo-t
Copy link
Contributor Author

horo-t commented Sep 2, 2021

I created a pull request #7008. Could you please review?

@domenic
Copy link
Member

domenic commented Sep 2, 2021

Another point in the design space that just occurred to me is to omit "classic" and use this only for types that have their own dedicated <script type=""> string. Since everyone can assume classic script support as a baseline.

I think I am OK either way, and maybe even slightly prefer keeping "classic" based on the arguments about symmetry with the Worker constructor, but I wanted to mention that possibility.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 3, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
@annevk
Copy link
Member

annevk commented Sep 3, 2021

Ah interesting; I'm okay with either.

@domenic
Copy link
Member

domenic commented Sep 3, 2021

@guybedford would this be useful to es-module-shims in any way? In particular it would be a simple synchronous way to detect importmap support.

@guybedford
Copy link
Contributor

Yes, it definitely would be useful. I wonder if this could extend to JSON / CSS module script support detections as well?

Just removing import maps from that list would be a huge benefit though.

@domenic
Copy link
Member

domenic commented Sep 3, 2021

I wonder if this could extend to JSON / CSS module script support detections as well?

I think that would require a different entry point, since they are all types of modules. Maybe something like import.meta.supportsType('css'). But I like the idea and wonder if we can get any implementer interest :). Probably worth a separate thread; I'll start one...

@domenic
Copy link
Member

domenic commented Sep 3, 2021

A separate note: I like putting this on HTMLScriptElement because it's specifically about what types are supported by <script>. E.g. you cannot use importmap or speculationrules in new Worker(url, { type }).

If we do ever extend workers with more types, we could introduce a separate Worker.supports('module').

@domenic domenic closed this as completed in 33ff054 Sep 3, 2021
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 6, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}
pull bot pushed a commit to Alan-love/chromium that referenced this issue Sep 6, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 22, 2021
…e) method, a=testonly

Automatic update from web-platform-tests
Implement HTMLScriptElement.supports(type) method

Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}

--

wpt-commits: e02f4a5db0a268cb888edf0abbf90eff3cd98d3a
wpt-pr: 30284
aosmond pushed a commit to aosmond/gecko that referenced this issue Sep 24, 2021
…e) method, a=testonly

Automatic update from web-platform-tests
Implement HTMLScriptElement.supports(type) method

Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}

--

wpt-commits: e02f4a5db0a268cb888edf0abbf90eff3cd98d3a
wpt-pr: 30284
Gabisampaio pushed a commit to Gabisampaio/wpt that referenced this issue Nov 18, 2021
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Bug: 1245528, whatwg/html#6472

Change-Id: I9a902504cf692caa73ae7e49fd65895156bbf197
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3133553
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918474}
NOKEYCHECK=True
GitOrigin-RevId: b8b24648e5716bd4e7f7cb77cb792163e8599149
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants
@guybedford @domenic @annevk @horo-t @hiroshige-g and others