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

Define "relevant settings object" for any platform object, not just global objects. #564

Closed
wants to merge 4 commits into from

Conversation

jyasskin
Copy link
Member

This grounds lookups like the ones in https://webbluetoothcg.github.io/web-bluetooth/#notification-events and https://webbluetoothcg.github.io/web-bluetooth/#service-change-events where I'm trying to get an event loop for an object.

Apologies if I've gotten any markup wrong.

(There is always a 1:1 mapping of global objects to environment settings objects.)</p>
<p>The <dfn>relevant settings object</dfn> for a <span>platform object</span> <var>o</var> is:</p>
<dl class="switch">
<dt>If <var>o</var> is a global object,</dt>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<p> and </p> inside <dt>s and <dd>s is the prevailing style, I am pretty sure...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent one space, not two (here and throughout)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dt doesn't allow p :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, seems like it does... But didn't before. Anyway source doesn't contain <dt><p>.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing comma in the dts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the indent; removed the <dl> entirely per a subsequent comment.

@domenic
Copy link
Member

domenic commented Jan 27, 2016

I haven't run the build yet, but it seems like changing the <dfn> from "relevant settings object for a global object" to "relevant settings object" will break most of the "call sites" that refer back to it...

<var>o</var>. (There is always a 1:1 mapping of global objects to environment settings
objects.)</dd>
<dt>Otherwise,</dt>
<dd>the <span>relevant settings object</span> for the global object of the <span>global
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just use the same phrasing as above: ... whose <span>global object</span> is the ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, but I think I like the recursive way better. It's more clear that the steps after getting to a global object are the same.

@jyasskin
Copy link
Member Author

@domenic, there are no uses of "relevant settings object" in HTML. I think the only uses are https://w3c.github.io/webappsec-secure-contexts/#framework and https://w3c.github.io/webappsec-mixed-content/#categorize-settings-object. (@mikewest)

@tabatkins Can Shepherd find all uses of a particular term?

@domenic
Copy link
Member

domenic commented Jan 27, 2016

Ah OK. In that case this LGTM, although I still haven't run the build (we really should get the CI set up...). @annevk, want to sign off too?

@annevk
Copy link
Member

annevk commented Jan 27, 2016

Yeah, I think this is alright. This term was added as a hook for XMLHttpRequest. So I guess we'd have to update the links everywhere or keep the old ID working.

@jyasskin
Copy link
Member Author

I can also squash and/or rebase when you're ready.

Updating the links I've found will generally make them shorter and simpler, but I can also add the extra <dfn data-x="relevant settings object for a global object"></dfn> (or another way to keep the old ID working?) next to the new one.

@annevk
Copy link
Member

annevk commented Jan 27, 2016

You can put an id attribute directly on the dfn. Let's do that since folks don't like it when we change them.

@domenic
Copy link
Member

domenic commented Jan 27, 2016

You can put an id attribute directly on the dfn. Let's do that since folks don't like it when we change them.

This does not work. The only thing that works is to add an empty span with an id nearby. I have tried this repeatedly.

@annevk
Copy link
Member

annevk commented Jan 27, 2016

Euhm, search for e.g., defineTimeline in the source and use it as fragment.

@jyasskin
Copy link
Member Author

The way defineTimeline works leads bikeshed specs to require "establishing the media timeline" as the link text, and link to https://html.spec.whatwg.org/multipage/embedded-content.html#defineTimeline. I think we want newly-built specs to link to #relevant-settings-object, and if we don't want to break the build for bikeshed specs, we also need a <dfn data-lt="relevant settings object for a global object"> in the final output. Here's a patch that does this.

<span>environment settings object</span> whose <span>global object</span> is <var>o</var>.
(There is always a 1:1 mapping of global objects to environment settings objects.)</p>
<p>The <dfn data-x="relevant settings object for a global object" data-lt="relevant settings
object for a global object"></dfn><dfn>relevant settings object</dfn> for a <span>global
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong. How about we put the old id on the <p> given that you want both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that Mixed Content and Secure Contexts already define their own shortcut for "relevant settings object", so omitting the data-lt won't break their builds. It probably will break https://github.com/whatwg/xhr/blob/master/Overview.src.html#L283. Done anyway.

…l-object working, instead of the full <dfn> to keep source compatibility.
annevk pushed a commit that referenced this pull request Jan 27, 2016
@annevk
Copy link
Member

annevk commented Jan 27, 2016

Thank you, landed as 25eaf88.

@annevk annevk closed this Jan 27, 2016
@jyasskin jyasskin deleted the relevant-settings-object branch December 18, 2018 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants