1132673 - Remove ServiceWorkerGlobalScope.scope

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Not in latest spec. Access is via underlying ServiceWorkerRegistration now (we don't support that yet).

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Need to keep the underlying mScope around so ServiceWorkerClients can use it, but we can get rid of the getter. This should be a quick first bug:
1) Remove the scope line in dom/webidl/ServiceWorkerGlobalScope.webidl.
2) Change GetScope's signature in dom/workers/WorkerScope.h to |void GetScope(nsString& aScope)| and body to |aScope = mScope|.

Comment 2

[Jason Gersztyn]

Hello there. I am new to open source, and have been having quite a bit of trouble finding a bug to work on. I would love to take this bug and work on it. Seems like it would be a great start!

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Jason, go ahead. I've already outlined how to fix it in comment 1.

Comment 4

[Jason Gersztyn]

(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #3)
> Jason, go ahead. I've already outlined how to fix it in comment 1.

Great! Thanks for outlining things so clearly. Seems like I already found the necessary files and will push a patch along soon.

Comment 5

[Jason Gersztyn]

Attachment: bug_1132673.diff

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8568160 [details] [diff] [review]
bug_1132673.diff

Patch is empty.

Comment 7

[Jason Gersztyn]

I see now and have attempted to make additional patches, but it still isn't working. I am certain the changes had been made, yet Mercurial is not acknowledging there are any differences. Even "hg status" says the files have not been edited. I have tried changing the code to its original state and then changing it back, but it still isn't acknowledging any changes have been made.

Is there something I can do to make sure the changes are noticed? I feel like a complete burden at this point, but please understand this is my first time trying to do anything like this. My apologies.

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

What does |hg diff| output after you make changes? It would be good if you could ping me on IRC (nsm) between 9am-5pm EST (Toronto) (for this week)

Comment 9

[Jason Gersztyn]

Attachment: bug_1132673_v2.diff

with a non-empty patch this time.

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8570702 [details] [diff] [review]
bug_1132673_v2.diff

Review of attachment 8570702 [details] [diff] [review]:
-----------------------------------------------------------------

Do you have level 1 commit access? In that case you can push to try, otherwise I'll land it for you.

Comment 11

[Jason Gersztyn]

I'm not entirely sure if it will let me. I was able to commit the changes, but when trying to push this is what happens:

jason@jason-HP-ENVY-dv6-Notebook-PC:~/mozilla-central$ hg push
pushing to ssh://[e-mail]/integration/b2g-inbound/
remote: Permission denied (publickey).
abort: no suitable response from remote hg!

Comment 12

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Your mercurial isn't set up properly. Please see https://developer.mozilla.org/en-US/docs/Installing_Mercurial

You don't have access, so I'll do it.

Comment 13

[Jason Gersztyn]

Well darn. I will get it working properly then. I appreciate you landing the patch. While it doesn't seem like a lot, it is quite exciting for me!

Comment 14

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Jason, you won't be allowed to push to anywhere except try initially. See https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/
You may apply for level 1 access to make it easier for you to keep contributing. Once you can push to try, you can just add the 'checkin-needed' keyword to the bug (in the keyword field) after you post a link to a tryserver build. Then some scripts will automatically land the patch.

Meanwhile, I can't land this patch just yet since we need sign-off from a dom peer. asking baku.

Thanks for the patch! It should land within a day or two.

Comment 15

[Jason Gersztyn]

Alright. Looking forward to having it land officially!

The link you have provided is also great deal of help. Thank you.

Comment 16

[Andrea Marchesini [:baku]]

Comment on attachment 8570702 [details] [diff] [review]
bug_1132673_v2.diff

Review of attachment 8570702 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Jason!

Comment 17

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/977e7f1f3927

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/977e7f1f3927