Support controlling interface and member visibility via preferences by jdm · Pull Request #10081 · servo/servo

jdm · 2016-03-18T17:15:43Z

This was easier to throw together than per-attribute/method support, and it gets rid of some nonstandard properties from our globals.

Fixes #7626.

r? @Ms2ger

This change is

jdm · 2016-03-18T17:17:53Z

I discovered that in the case of:

[Pref="foo"]
interface Parent {}

interface Child : Parent {}

Parent still gets defined in the current implementation even if foo is false. What should we do in this case?

nox · 2016-03-19T15:21:51Z

Do you mean that Child still gets defined? Anyway whether it is parent or child I don't think either of them should be visible.

jdm · 2016-03-19T21:18:06Z

No, I mean that window.Parent is still defined even if the preference controlling it's visibility is false.

nox · 2016-03-20T10:18:02Z

Well that's obviously wrong, no?

larsbergstrom · 2016-03-23T19:23:39Z

👍 to `[Pref="dom.bluetooth.enabled"], and off by default.

cc @dati91

jdm · 2016-03-23T21:15:46Z

This is ready for review.

jdm · 2016-03-24T03:45:10Z

This is still ready for review; I just got carried away and implemented the Pref annotation for members of interfaces as well.

bors-servo · 2016-03-26T20:15:50Z

☔ The latest upstream changes (presumably #10204) made this pull request unmergeable. Please resolve the merge conflicts.

paulrouget · 2016-03-29T16:50:53Z

Can we get a review on that?

jdm · 2016-04-27T22:56:49Z

This has been ignored for more than a month. I would really appreciate if someone would take responsibility for reviewing my work :(

nox · 2016-04-29T09:04:21Z

On it.

Could you give more details about what you meant by "This was easier to throw together than per-attribute/method support"?

nox · 2016-04-29T09:06:42Z

@jdm I think you are cursed.

pkaminski · 2016-04-29T09:15:51Z

Hey @jdm, how did you make that happen? Reviewable thinks that somehow there's a discussion attached to a file that doesn't exist in the review, hence boom.

Comments from Reviewable

pkaminski · 2016-04-29T09:17:02Z

I mean @nox, sorry.

Comments from Reviewable

nox · 2016-04-29T09:18:18Z

@pkaminski I just clicked on the Reviewable button and it failed to load.

@jdm I don't get the purpose of avoiding JS_DefineProperties and friends, they just loop over items and call JS_DefineProperty sequentially. Couldn't we just rewrite JS_DefineProperties ourselves, putting a new field in JSPropertySpec or a similar structure?

nox · 2016-04-29T09:19:08Z

components/script/dom/bindings/codegen/CodegenRust.py

+                    properties[arrayName] = "EMPTY_CONSTANTS"
                else:
-                    properties[arrayName] = "&[]"
+                    properties[arrayName] = array.variableName()


Please revert this. It worked before and you didn't even document EMPTY_CONSTANTS.

highfive · 2016-05-02T18:19:31Z

  ▶ Unexpected subtest result in /_mozilla/mozilla/preferences.html:
  │ FAIL [expected PASS] prefs
  │   → TestBinding is not defined
  │ 
  │ @http://web-platform.test:8000/_mozilla/mozilla/preferences.html:2:6
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1397:20
  │ test@http://web-platform.test:8000/resources/testharness.js:495:9
  └ @http://web-platform.test:8000/_mozilla/mozilla/preferences.html:1:1

…ally-exposed ones.

…nce checks.

…opriate prefs.

jdm · 2016-05-02T18:33:29Z

@bors-servo: r=nox

bors-servo · 2016-05-02T18:33:30Z

📌 Commit 2a9bf70 has been approved by nox

bors-servo · 2016-05-02T18:35:39Z

⌛ Testing commit 2a9bf70 with merge 8255e74...

@Ms2ger

Support controlling interface and member visibility via preferences This was easier to throw together than per-attribute/method support, and it gets rid of some nonstandard properties from our globals. Fixes #7626. r? @Ms2ger  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10081)

bors-servo · 2016-05-02T19:45:34Z

💔 Test failed - linux-rel

highfive · 2016-05-02T19:45:36Z

  ▶ FAIL [expected PASS] /_mozilla/css/iframe/mozbrowser_navigation.html
  └   → /_mozilla/css/iframe/mozbrowser_navigation.html e60785ff75153e5d215bd0f65141fbb13cfbc61b
/_mozilla/css/iframe/mozbrowser_navigation_ref.html 8250706e506ad24231cc23be315b9b868387c598
Testing e60785ff75153e5d215bd0f65141fbb13cfbc61b == 8250706e506ad24231cc23be315b9b868387c598

cbrewster · 2016-05-02T19:57:27Z

@bors-servo retry

FAIL [expected PASS] /_mozilla/css/iframe/mozbrowser_navigation.html #10924

bors-servo · 2016-05-02T20:00:24Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

bors-servo · 2016-05-02T20:40:42Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

bors-servo · 2016-05-02T20:46:28Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned Ms2ger Mar 18, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 18, 2016

paulrouget mentioned this pull request Mar 21, 2016

forcetouch events #9811

Merged

larsbergstrom mentioned this pull request Mar 23, 2016

Implement WebBluetooth in Servo #9392

Closed

jdm force-pushed the interfacepref branch from e8f3b2e to 9afd906 Compare March 23, 2016 21:15

jdm force-pushed the interfacepref branch from 41a29cc to 9cb157e Compare March 23, 2016 21:17

jdm changed the title ~~Support controlling interface visibility via preferences~~ Support controlling interface and member visibility via preferences Mar 24, 2016

jdm force-pushed the interfacepref branch 2 times, most recently from 83e93ac to e06f60d Compare March 24, 2016 14:03

highfive added the S-needs-rebase There are merge conflict errors. label Mar 26, 2016

jdm mentioned this pull request Apr 13, 2016

Implement non-visible pipeline and iframe visibility methods #10225

Merged

nox assigned nox and unassigned Ms2ger Apr 18, 2016

nox mentioned this pull request Apr 29, 2016

Unexpected crash on Servo PR Reviewable/Reviewable#299

Closed

nox reviewed Apr 29, 2016
View reviewed changes

jdm added 7 commits May 2, 2016 14:32

Disable nonstandard interfaces via prefs.

91a78ed

Forbid unconditionally-exposed interfaces that inherit from condition…

f4c1529

…ally-exposed ones.

Ensure bluetooth and testbinding are disabled by default.

693d31d

Start generating arrays of specs for easier implementation of prefere…

88059ac

…nce checks.

Implement hiding of interface members via Pref annotations.

cb5bad6

Hide nonstandard interface members of standard interfaces behind appr…

a03fc5e

…opriate prefs.

Move FocusEvent into the right list of exposed interfaces.

2a9bf70

jdm force-pushed the interfacepref branch from 48b2b50 to 2a9bf70 Compare May 2, 2016 18:33

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels May 2, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels May 2, 2016

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 2, 2016

highfive added the S-tests-failed The changes caused existing tests to fail. label May 2, 2016

bors-servo merged commit 2a9bf70 into servo:master May 2, 2016

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label May 2, 2016

This was referenced May 2, 2016

WebBluetooth impementation #10632

Merged

Issue #10605 generate unions from Typdefs inner type #10615

Closed

jdm mentioned this pull request May 20, 2016

Implement window.openURLInDefaultBrowser() #11292

Closed

Uh oh!

Conversation

jdm commented Mar 18, 2016

Uh oh!

jdm commented Mar 18, 2016

Uh oh!

nox commented Mar 19, 2016

Uh oh!

jdm commented Mar 19, 2016

Uh oh!

nox commented Mar 20, 2016

Uh oh!

larsbergstrom commented Mar 23, 2016

Uh oh!

jdm commented Mar 23, 2016

Uh oh!

jdm commented Mar 24, 2016

Uh oh!

bors-servo commented Mar 26, 2016

Uh oh!

paulrouget commented Mar 29, 2016

Uh oh!

jdm commented Apr 27, 2016

Uh oh!

nox commented Apr 29, 2016

Uh oh!

nox commented Apr 29, 2016

Uh oh!

pkaminski commented Apr 29, 2016

Uh oh!

pkaminski commented Apr 29, 2016

Uh oh!

nox commented Apr 29, 2016

Uh oh!

nox Apr 29, 2016

Choose a reason for hiding this comment

Uh oh!

highfive commented May 2, 2016

Uh oh!

jdm commented May 2, 2016

Uh oh!

bors-servo commented May 2, 2016

Uh oh!

bors-servo commented May 2, 2016

Uh oh!

bors-servo commented May 2, 2016

Uh oh!

highfive commented May 2, 2016

Uh oh!

cbrewster commented May 2, 2016 • edited by jdm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors-servo commented May 2, 2016

Uh oh!

bors-servo commented May 2, 2016

Uh oh!

bors-servo commented May 2, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

cbrewster commented May 2, 2016 •

edited by jdm

Loading