Remove ServiceWorkerGlobalScope.scope

RESOLVED FIXED in Firefox 39

Status

()

defect
RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: nsm, Assigned: jgersztyn13, Mentored)

Tracking

33 Branch
mozilla39
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

(Whiteboard: [good first bug][lang=C++])

Attachments

(1 attachment, 1 obsolete attachment)

Not in latest spec. Access is via underlying ServiceWorkerRegistration now (we don't support that yet).
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|.
Mentor: nsm.nikhil
Whiteboard: [good first bug][lang=C++]
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!
Jason, go ahead. I've already outlined how to fix it in comment 1.
Assignee: nobody → jgersztyn13
(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.
Posted patch bug_1132673.diff (obsolete) — Splinter Review
Attachment #8568160 - Flags: review?(nsm.nikhil)
Comment on attachment 8568160 [details] [diff] [review]
bug_1132673.diff

Patch is empty.
Attachment #8568160 - Flags: review?(nsm.nikhil)
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.
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)
with a non-empty patch this time.
Attachment #8568160 - Attachment is obsolete: true
Attachment #8570702 - Flags: review?(nsm.nikhil)
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.
Attachment #8570702 - Flags: review?(nsm.nikhil) → review+
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://user%40host.name@hg.mozilla.org/integration/b2g-inbound/
remote: Permission denied (publickey).
abort: no suitable response from remote hg!
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.
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!
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.
Attachment #8570702 - Flags: review+ → review?(amarchesini)
Alright. Looking forward to having it land officially!

The link you have provided is also great deal of help. Thank you.
Comment on attachment 8570702 [details] [diff] [review]
bug_1132673_v2.diff

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

Thanks Jason!
Attachment #8570702 - Flags: review?(amarchesini) → review+
https://hg.mozilla.org/mozilla-central/rev/977e7f1f3927
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.