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

Get rid of entry settings object usage #168

Merged
merged 2 commits into from
Apr 27, 2021
Merged

Get rid of entry settings object usage #168

merged 2 commits into from
Apr 27, 2021

Conversation

annevk
Copy link
Member

@annevk annevk commented Apr 22, 2021

Use the relevant settings object instead.

Fixes #86.

(Note that this builds upon #167.)


Preview | Diff

@annevk annevk requested a review from domenic April 22, 2021 13:47
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Statics are problematic

notifications.bs Outdated Show resolved Hide resolved
notifications.bs Outdated
@@ -718,7 +722,7 @@ constructor steps are:
<h3 id=static-members>Static members</h3>

<p>The static <dfn attribute for=Notification><code>permission</code></dfn> getter steps are to
return the <a>permission</a> for the <a>entry settings object</a>'s
return the <a>permission</a> for <a>this</a>'s <a>relevant settings object</a>'s
Copy link
Member

Choose a reason for hiding this comment

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

This is static so it doesn't work. You want the <a>current settings object</a>.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm this probably is preexisting in requestPermission() somehow.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fixed both, thanks for spotting that!

notifications.bs Show resolved Hide resolved
Base automatically changed from annevk/idl to main April 26, 2021 06:32
Use the relevant settings object instead.

Fixes #86.
@annevk
Copy link
Member Author

annevk commented Apr 26, 2021

New commit message:

Get rid of entry settings object usage

Use the relevant global/settings object where possible; use the current global/settings object in statics.

Fixes #86.

@annevk annevk requested a review from domenic April 26, 2021 06:50
@annevk annevk merged commit 0ed9d85 into main Apr 27, 2021
@annevk annevk deleted the annevk/entry branch April 27, 2021 05:23
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.

Remove usage of "entry settings object"
2 participants