1689565 - Remove Notification.get()

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect, P5)

Description

[Philip Jägenstedt [:foolip]]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 11_1_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.96 Safari/537.36

Steps to reproduce:

Have a look at https://github.com/mozilla/gecko-dev/blob/06dbf167c94b89fb06f74a2d1b1c5d3ccabceb92/dom/webidl/Notification.webidl#L27

Actual results:

Gecko still has a Notification.get() static method.

Expected results:

This should be removed. It was removed from the spec way back in https://github.com/whatwg/notifications/commit/11f25cccb909adc7e8b5569f8e5e20c482234860.

Comment 1

[Anne (:annevk)]

Basic test: https://github.com/web-platform-tests/wpt/pull/27385.

Comment 2

[Ho Cheung]

Can I help to fix this bug?

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Yup, please do!

Refer to https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html if you are not familiar with the things yet. Submitting a patch via moz-phab will automatically assign this to you.

Comment 4

[Ho Cheung]

Attachment: WIP: Bug 1689565 - Remove Notification.get()

Remove Notification.get() in the code

Comment 5

[Ho Cheung]

I have submitted a patch for this issue, please review

https://phabricator.services.mozilla.com/D167145

Comment 6

[Ho Cheung]

Attachment: WIP: Bug 1689565 - Remove Notification.get()

Remove Notification.get() in the code

Depends on D167145

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

Make sure you get everything in a single commit 😉

Comment 8

[Kagami Rosylight [:saschanaz] (they/them)]

(Multiple commits are fine, but only when it's intended, not just some changes to the first commit)

Comment 9

[Ho Cheung]

Attachment: Bug 1689565 - Part 1: Remove Notification.get() r=saschanaz

Remove Notification.get() in the code

Comment 10

[Ho Cheung]

Due to issues with the previous commit, a new commit has now been initiated, please review

https://phabricator.services.mozilla.com/D167236

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1689565 - Part 3: Migrate removed tests r=#dom-worker-reviewers

Depends on D167236

Comment 12

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 1689565 - Part 2: Tweak marionette hack to allow `notifications` r=jgraham

Depends on D167236

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c51d687162d5
Part 1: Remove Notification.get() r=saschanaz
https://hg.mozilla.org/integration/autoland/rev/66af1d60952f
Part 2: Tweak marionette hack to allow `notifications` r=jgraham,webdriver-reviewers
https://hg.mozilla.org/integration/autoland/rev/58172efe03db
Part 3: Migrate removed tests r=dom-worker-reviewers,smaug

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/38256 for changes under testing/web-platform/tests

Comment 15

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/c51d687162d5
https://hg.mozilla.org/mozilla-central/rev/66af1d60952f
https://hg.mozilla.org/mozilla-central/rev/58172efe03db

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot