Add SupportedPropertyNames to Document (also fix iframe getting) #25548

pshaughn · 2020-01-17T23:56:43Z

Existing test of named-getting an iframe now succeeds. I added a new test for Object.getOwnPropertyNames(document) based on my understanding of the spec; that test could use a second opinion.

UPDATE: This was trying to do too many things in one PR as originally submitted. It is now using #25572 as a base, and I suggest reviewing that PR before this one to avoid duplicating review effort.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #7273 for all implemented named getters, fix #25146, and fix the iframe case only of #25145.

There are tests for these changes

highfive · 2020-01-17T23:56:49Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/document.rs, components/script/dom/shadowroot.rs, components/script/dom/element.rs, components/script/dom/raredata.rs, components/script/dom/documentorshadowroot.rs
@KiChjang: components/script/dom/document.rs, components/script/dom/shadowroot.rs, components/script/dom/element.rs, components/script/dom/raredata.rs, components/script/dom/documentorshadowroot.rs

highfive · 2020-01-17T23:56:52Z

Warning

These commits modify unsafe code. Please review it carefully!

servo-wpt-sync · 2020-01-17T23:57:02Z

Opened new PR for upstreamable changes.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21247.


        Auto merge of #25562 - pshaughn:window-getter, r=<try>

Attempt at window named getter, sharing name_map infrastructure with document named getter I think I have the window named getter working well enough to look at CI results. This has #25548 as a base but also pulls in most of #21869.


        Auto merge of #25562 - pshaughn:window-getter, r=<try>

Attempt at window named getter, sharing name_map infrastructure with document named getter I think I have the window named getter working well enough to look at CI results. This has #25548 as a base but also pulls in most of #21869.


        Auto merge of #25562 - pshaughn:window-getter, r=<try>

Attempt at window named getter, sharing name_map infrastructure with document named getter I think I have the window named getter working well enough to look at CI results. This has #25548 as a base but also pulls in most of #21869.

pshaughn · 2020-01-21T15:58:37Z

This is broken right now and needs #25570 fixed as a foundation so element names don't sneak out of the atom cache.

servo-wpt-sync · 2020-01-22T21:10:25Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21247.

pshaughn · 2020-01-22T21:20:04Z

Now using #25572 as a base, which should prevent crashes. I also took into account review comments by @Manishearth from #25562 (which I plan on updating to use this as a base, if this seems solid enough.)
@bors-servo try

bors-servo · 2020-01-22T21:20:12Z

⌛ Trying commit 7a2456a with merge 2cbc26e...


        Auto merge of #25548 - pshaughn:docnamedgetter, r=<try>

Add SupportedPropertyNames to Document (also fix iframe getting) Existing test of named-getting an iframe now succeeds. I added a new test for Object.getOwnPropertyNames(document) based on my understanding of the spec; that test could use a second opinion. UPDATE: This was trying to do too many things in one PR as originally submitted. It is now using #25572 as a base, and I suggest reviewing that PR before this one to avoid duplicating review effort. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #7273 for all implemented named getters, fix #25146, and fix the iframe case only of #25145.  - [X] There are tests for these changes

pshaughn · 2020-01-22T21:23:52Z

(forgot to account for comment #25562 (comment), so as of this push the new supported property names test is still broken out into a bunch of really small cases)

bors-servo · 2020-01-22T22:41:11Z

☀️ Test successful - status-taskcluster
State: approved= try=True

jdm

Great work! I found this to be quite readable, which is better than the attempt that I made at it originally!

jdm · 2020-02-12T16:08:15Z

This can merge after a rebase and #25572 is merged.


        Names should now be consistently atoms


        Doc named getter improvements

servo-wpt-sync · 2020-02-13T21:04:06Z

Transplanted upstreamable changes to existing PR.

Completed upstream sync of web-platform-test changes at web-platform-tests/wpt#21247.

jdm · 2020-02-13T21:59:52Z

@bors-servo r+

bors-servo · 2020-02-13T21:59:53Z

📌 Commit e48eac6 has been approved by jdm


        Auto merge of #25562 - pshaughn:window-getter, r=<try>

Attempt at window named getter, sharing name_map infrastructure with document named getter I think I have the window named getter working well enough to look at CI results. This has #25548 as a base but also pulls in most of #21869.


        Auto merge of #25562 - pshaughn:window-getter, r=<try>

Attempt at window named getter, sharing name_map infrastructure with document named getter I think I have the window named getter working well enough to look at CI results. This has #25548 as a base but also pulls in most of #21869.


        Auto merge of #25548 - pshaughn:docnamedgetter, r=jdm

Add SupportedPropertyNames to Document (also fix iframe getting) Existing test of named-getting an iframe now succeeds. I added a new test for Object.getOwnPropertyNames(document) based on my understanding of the spec; that test could use a second opinion. UPDATE: This was trying to do too many things in one PR as originally submitted. It is now using #25572 as a base, and I suggest reviewing that PR before this one to avoid duplicating review effort. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #7273 for all implemented named getters, fix #25146, and fix the iframe case only of #25145.  - [X] There are tests for these changes

bors-servo · 2020-02-13T22:57:18Z

⌛ Testing commit e48eac6 with merge f59f110...

bors-servo · 2020-02-13T23:44:57Z

💔 Test failed - status-taskcluster

jdm · 2020-02-13T23:55:45Z

@bors-servo retry

#25513

bors-servo · 2020-02-14T01:11:22Z

⌛ Testing commit e48eac6 with merge f020536...


        Auto merge of #25548 - pshaughn:docnamedgetter, r=jdm

Add SupportedPropertyNames to Document (also fix iframe getting) Existing test of named-getting an iframe now succeeds. I added a new test for Object.getOwnPropertyNames(document) based on my understanding of the spec; that test could use a second opinion. UPDATE: This was trying to do too many things in one PR as originally submitted. It is now using #25572 as a base, and I suggest reviewing that PR before this one to avoid duplicating review effort. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #7273 for all implemented named getters, fix #25146, and fix the iframe case only of #25145.  - [X] There are tests for these changes

bors-servo · 2020-02-14T01:23:32Z

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing f020536 to master...

highfive assigned asajeffrey Jan 17, 2020

highfive added the S-awaiting-review label Jan 17, 2020

servo-wpt-sync mentioned this pull request Jan 17, 2020

Add SupportedPropertyNames to Document (also fix iframe getting) web-platform-tests/wpt#21247

Merged

This was referenced Jan 20, 2020

Document named getter can't find embeds and objects #25145

Open

Attempt at window named getter, sharing name_map infrastructure with document named getter #25562

Draft

pshaughn mentioned this pull request Jan 21, 2020

Handle element names consistently #25570

Closed

pshaughn force-pushed the pshaughn:docnamedgetter branch from e20b9bc to 7a2456a Jan 22, 2020

jdm approved these changes Feb 12, 2020

View changes

jdm added S-needs-rebase S-blocked-on-external and removed S-awaiting-review labels Feb 12, 2020

jdm assigned jdm and unassigned asajeffrey Feb 12, 2020

pshaughn added 2 commits Jan 21, 2020

Names should now be consistently atoms

4b750ca

Doc named getter improvements

Loading status checks…

e48eac6

pshaughn force-pushed the pshaughn:docnamedgetter branch from 7a2456a to e48eac6 Feb 13, 2020

highfive added the S-awaiting-review label Feb 13, 2020

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-rebase labels Feb 13, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Feb 13, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Feb 14, 2020

bors-servo merged commit e48eac6 into servo:master Feb 14, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Feb 14, 2020

bors-servo mentioned this pull request Feb 14, 2020

[WIP] Named window getter #21869

Open

servo / servo

Add SupportedPropertyNames to Document (also fix iframe getting) #25548

Add SupportedPropertyNames to Document (also fix iframe getting) #25548

pshaughn commented Jan 17, 2020 •

edited

highfive commented Jan 17, 2020

highfive commented Jan 17, 2020

servo-wpt-sync commented Jan 17, 2020

pshaughn commented Jan 21, 2020

servo-wpt-sync commented Jan 22, 2020

pshaughn commented Jan 22, 2020

bors-servo commented Jan 22, 2020

pshaughn commented Jan 22, 2020

bors-servo commented Jan 22, 2020

jdm left a comment

jdm commented Feb 12, 2020

servo-wpt-sync commented Feb 13, 2020

jdm commented Feb 13, 2020

bors-servo commented Feb 13, 2020

bors-servo commented Feb 13, 2020

bors-servo commented Feb 13, 2020

jdm commented Feb 13, 2020

bors-servo commented Feb 14, 2020

bors-servo commented Feb 14, 2020

servo / servo

Join GitHub today

Add SupportedPropertyNames to Document (also fix iframe getting) #25548

Add SupportedPropertyNames to Document (also fix iframe getting) #25548

Conversation

pshaughn commented Jan 17, 2020 • edited

highfive commented Jan 17, 2020

highfive commented Jan 17, 2020

servo-wpt-sync commented Jan 17, 2020

pshaughn commented Jan 21, 2020

servo-wpt-sync commented Jan 22, 2020

pshaughn commented Jan 22, 2020

bors-servo commented Jan 22, 2020

pshaughn commented Jan 22, 2020

bors-servo commented Jan 22, 2020

jdm left a comment

jdm commented Feb 12, 2020

servo-wpt-sync commented Feb 13, 2020

jdm commented Feb 13, 2020

bors-servo commented Feb 13, 2020

bors-servo commented Feb 13, 2020

bors-servo commented Feb 13, 2020

jdm commented Feb 13, 2020

bors-servo commented Feb 14, 2020

bors-servo commented Feb 14, 2020

pshaughn commented Jan 17, 2020 •

edited