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

Stop using T[] (webidl arrays) #11

Closed
domenic opened this issue Aug 28, 2015 · 12 comments

Comments

5 participants
@domenic
Copy link
Member

commented Aug 28, 2015

In heycam/webidl@079cbb8 the T[] Web IDL type was removed.

After #10, the remaining uses in the spec of T[] will be:

I haven't looked yet whether these can be completely replaced with FrozenArray or whether something trickier might be needed.

@annevk annevk referenced this issue Jan 11, 2016

Closed

Remove uses of webidl arrays. #497

0 of 4 tasks complete

@annevk annevk changed the title Stop using T[] Stop using T[] (webidl arrays) Jan 11, 2016

@foolip

This comment has been minimized.

Copy link
Member

commented Mar 11, 2016

I took a look but none of these are entirely trivial.

MessageEvent#ports is nullable, does that really make sense? If it's just for the constructor, maybe it should return an empty array instead of null?

DataTransfer#types says "The types attribute must return a live read only array giving the strings that the following steps would produce." The important bit seems to be that the array becomes empty "If the DataTransfer object is no longer associated with a drag data store".

Location#ancestorOrigins is defined in such a way that it's not obvious that it always return the same list.

NavigatorLanguage#languages can change at runtime if the browser settings change.

For the last three, it seems like what we need is a variation of FrozenArray that can be updated at specific points, but stays constant otherwise.

@domenic

This comment has been minimized.

Copy link
Member Author

commented Mar 11, 2016

For the last three, it seems like what we need is a variation of FrozenArray that can be updated at specific points, but stays constant otherwise.

That is exactly FrozenArray :). You replace the object at specific points with a new FrozenArray.

@foolip

This comment has been minimized.

Copy link
Member

commented Mar 11, 2016

I have been confused, see whatwg/notifications#66 (comment)

foolip added a commit that referenced this issue Mar 11, 2016

Use FrozenArray for location.ancestorOrigins and drop [SameObject]
[SameObject] only makes sense if the same array is returned every time,
but it can't be as browser context nesting can change:
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3988

Part of #11

foolip added a commit that referenced this issue Mar 11, 2016

Use FrozenArray for Navigator#languages and MessageEvent#ports
It is already clearly defined when the return value may change. For
languages, "The same object must be returned until the user agent needs
to return different values, or values in a different order." For ports,
"The ports attribute must return the value it was initialised to."

However, [SameObject] cannot be used for ports because it is nullable,
and "The [SameObject] extended attribute must not be used on anything
other than a read only attribute whose type is an interface type or
object."

Part of #11
@foolip

This comment has been minimized.

Copy link
Member

commented Mar 11, 2016

In this issue I would also include making sure that <dfn data-noexport="" data-x="dfn-read-only-array">Read only</dfn> (when applied to arrays) ends up unused and is removed.

foolip added a commit that referenced this issue Mar 14, 2016

foolip added a commit that referenced this issue Mar 16, 2016

foolip added a commit that referenced this issue Mar 16, 2016

foolip added a commit that referenced this issue Mar 24, 2016

Use FrozenArray for Navigator#languages and MessageEvent#ports
[SameObject] cannot be used for ports because it is nullable, and "The
[SameObject] extended attribute must not be used on anything other than
a read only attribute whose type is an interface type or object."

It also can't be used for languages, simply because it can change.

Part of #11

Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=23176
@foolip

This comment has been minimized.

Copy link
Member

commented Mar 24, 2016

Only DataTransfer#types left, and with that the "read only" term from WebIDL can be dropped.

@foolip

This comment has been minimized.

Copy link
Member

commented Mar 24, 2016

DataTransfer#types looks like it'll take some testing and investigation, so I don't plan to do that right now.

@zcorpan

This comment has been minimized.

Copy link
Member

commented Jun 7, 2016

cc @hallvors for DataTransfer#types

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2016

So I looked at what some UAs implement for DataTransfer#types:

  • Gecko implements it as a DOMStringList. Each get returns a new DOMStringList object.
  • Blink implements it as an Array (not frozen). Each get returns a new Array object.
  • WebKit does the same as Blink.
  • Edge does the same as Gecko.

Plausible paths forward:

  1. Spec the Blink/WebKit behavior. The "each get returns a new Array object" part is sucky, of course, plus the same compat issues as option 2.
  2. Spec a FrozenArray that gets recreated whenever the list of types changes. Seems like this would be fairly compatible, given the above. Neither Blink nor WebKit have a .contains or .item on their return value here, so it might be compatible enough for Gecko and Edge to drop them.
  3. Spec a DOMStringList. This has possible compat issues for Blink/WebKit to the extent that it doesn't have various Array stuff (e.g. filter() or indexOf on it). Of course Gecko and Edge don't have that right now either.

Thoughts? @domenic @foolip @annevk @smaug----

@foolip

This comment has been minimized.

Copy link
Member

commented Sep 23, 2016

FWIW, https://w3c.github.io/IndexedDB/#domstringlist now exists. @inexorabletash

FrozenArray seems like the nicest outcome and also likely to be web compatible, but I'm not sure if the bindings for having it change would be tricky. @bashi?

@bzbarsky

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2016

For what it's worth, they aren't very tricky in Gecko. Note that "change" means "forget the array object and start returning a new one now".

@inexorabletash

This comment has been minimized.

Copy link
Member

commented Sep 25, 2016

I doc'd DOMStringList in IDB just to have something describing what implementations currently have, since we're apparently not rushing towards FrozenArrayWithContains<DOMString> as quickly as we'd like.

Per w3c/IndexedDB#85 and F2F discussions, there's desire to make DOMStringList iterable<DOMString>, which should still be web-compatible with removing it.

@domenic

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2016

Plausible paths forward:

(2), a FrozenArray, sounds great, given that we don't have .contains or .item. My original concern was that I thought DataTransfer's types might change too frequently, but it sounds like it doesn't.

@domenic domenic self-assigned this Oct 4, 2016

domenic added a commit that referenced this issue Oct 4, 2016

Make DataTransfer's types attribute use a FrozenArray
This removes the last use of old Web IDL "read only arrays" (i.e. the DOMString[] syntax), thus closing #11.

zcorpan added a commit that referenced this issue Oct 5, 2016

Make DataTransfer's types attribute use a FrozenArray
This removes the last use of old Web IDL "read only arrays" (i.e. the DOMString[] syntax), thus closing #11.

@zcorpan zcorpan closed this Oct 5, 2016

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue May 16, 2017

DataTransfer: Make |types| be a FrozenArray<DOMString>.
Fix a TODO item: whatwg/html#11 was solved several
months ago by
whatwg/html@466379a,
so we can finally use a FrozenArray here.

Due to the way |types| is spec'ed, we need to use the [CachedAttribute]
custom extended attribute so that we can tell the bindings layer when a new
FrozenArray must be returned.

Since the data store item list can change via calls to DataTransfer's
setData() and DataTransfer.clearData(), as well as DataTransfer.items'
add(), remove() clear(), DataTransfer itself is notified of changes via the
newly-added DataObject::Observer whenever DataObject's underlying
|item_list_| is modified.

A new test has been added to verify this behavior.

BUG=652815
R=foolip@chromium.org,jsbell@chromium.org

Review-Url: https://codereview.chromium.org/2875013002
Cr-Commit-Position: refs/heads/master@{#472072}

scheib pushed a commit to scheib/chromium that referenced this issue May 16, 2017

DataTransfer: Make |types| be a FrozenArray<DOMString>.
Fix a TODO item: whatwg/html#11 was solved several
months ago by
whatwg/html@466379a,
so we can finally use a FrozenArray here.

Due to the way |types| is spec'ed, we need to use the [CachedAttribute]
custom extended attribute so that we can tell the bindings layer when a new
FrozenArray must be returned.

Since the data store item list can change via calls to DataTransfer's
setData() and DataTransfer.clearData(), as well as DataTransfer.items'
add(), remove() clear(), DataTransfer itself is notified of changes via the
newly-added DataObject::Observer whenever DataObject's underlying
|item_list_| is modified.

A new test has been added to verify this behavior.

BUG=652815
R=foolip@chromium.org,jsbell@chromium.org

Review-Url: https://codereview.chromium.org/2875013002
Cr-Commit-Position: refs/heads/master@{#472072}

foolip added a commit to web-platform-tests/wpt that referenced this issue May 17, 2017

DataTransfer: Make |types| be a FrozenArray<DOMString>.
Fix a TODO item: whatwg/html#11 was solved several
months ago by
whatwg/html@466379a,
so we can finally use a FrozenArray here.

Due to the way |types| is spec'ed, we need to use the [CachedAttribute]
custom extended attribute so that we can tell the bindings layer when a new
FrozenArray must be returned.

Since the data store item list can change via calls to DataTransfer's
setData() and DataTransfer.clearData(), as well as DataTransfer.items'
add(), remove() clear(), DataTransfer itself is notified of changes via the
newly-added DataObject::Observer whenever DataObject's underlying
|item_list_| is modified.

A new test has been added to verify this behavior.

BUG=652815
R=foolip@chromium.org,jsbell@chromium.org

Review-Url: https://codereview.chromium.org/2875013002
Cr-Commit-Position: refs/heads/master@{#472072}

Johanna-hub pushed a commit to Johanna-hub/html that referenced this issue Oct 22, 2018

Add scripts for deploying via Travis CI
This adds a new subdirectory, ci-deploy, with scripts and other
resources for building, validating, and deploying the spec entirely on
Travis. Previously Travis was only running build and validation steps,
but then throwing away the results, letting custom deploy architecture
handle the actual deploy. This replaces the custom deploy architecture
with something in source control and web-host-agnostic.

This deploy is Docker-based, for two main reasons:

* It is not easy to install FreePascal 3.x onto Travis CI's default
  Ubuntu configuration, which is fairly old (even with recent Trusty
  updates). It is simpler to create a Docker container with a recent OS
  and install FreePascal 3.x there.
* Docker has a good mechanism for caching previous results and not doing
  unnecessary work until the cache is invalidated. This is important as
  it allows us to avoid reinstalling prerequisite packages and
  recompiling Wattsi on every build. Travis CI has some caching support,
  but it is not as full-featured as Docker's. The intermediate
  containers created for this caching are stored on Docker Hub; see
  https://hub.docker.com/r/whatwg/html-deploy.

The ci-deploy/README.md file contains more instructions on how this is
expected to be used, and will be used by whatwg/html's .travis.yml in
whatwg#2941.

Fixes whatwg#11. Fixes whatwg#99. Fixes whatwg#103.

alice added a commit to alice/html that referenced this issue Jan 8, 2019

Make DataTransfer's types attribute use a FrozenArray
This removes the last use of old Web IDL "read only arrays" (i.e. the DOMString[] syntax), thus closing whatwg#11.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.