Rename {get,set,delete}{Window,Global}Value to match {get,set,delete}CustomTabValue

RESOLVED FIXED in Firefox 64

Status

()

P3
normal
RESOLVED FIXED
a year ago
6 months ago

People

(Reporter: dao, Assigned: jason, Mentored)

Tracking

Trunk
Firefox 64
Points:
---

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

a year ago
In bug 1446719 we renamed getTabValue, setTabValue and deleteTabValue. For consistency and clarity we should do the same for: getWindowValue, setWindowValue, deleteWindowValue, getGlobalValue, setGlobalValue, deleteGlobalValue.
Assignee: nobody → 1991manish.kumar
(Reporter)

Comment 2

a year ago
Seems correct!
Flags: needinfo?(dao+bmo)
How can I test this after changes on my system?
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 4

9 months ago
(In reply to Manish Kumar [:manishkk] from comment #3)
> How can I test this after changes on my system?

By running tests:

./mach mochitest browser/components/sessionstore/test/
./mach mochitest browser/components/extensions/test/browser/browser_ext_sessions_window_tab_value.js
Flags: needinfo?(dao+bmo)
Assignee: 1991manish.kumar → nobody
(Assignee)

Comment 5

6 months ago
Hi Dão - may I take this one?
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 6

6 months ago
Sure!
Assignee: nobody → jason
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 7

6 months ago
Posted patch bug-1451715.patch (obsolete) — Splinter Review
Requested changes made, tests ran without fail. Hope it looks good for you.
Thanks,
Jason
Flags: needinfo?(dao+bmo)
Attachment #9011032 - Flags: review+
(Reporter)

Updated

6 months ago
Flags: needinfo?(dao+bmo)
Attachment #9011032 - Flags: review+ → review?(dao+bmo)
(Reporter)

Comment 8

6 months ago
Comment on attachment 9011032 [details] [diff] [review]
bug-1451715.patch

This looks good, except that we only want to change the internal SessionStore API. The extension API shouldn't change for the time being.
Attachment #9011032 - Flags: review?(dao+bmo)
(Assignee)

Comment 9

6 months ago
Posted patch bug-1451715.patch (obsolete) — Splinter Review
Undid changes to session extension. Please review thanks!
Attachment #9011032 - Attachment is obsolete: true
Flags: needinfo?(dao+bmo)
Attachment #9011898 - Flags: review+
(Reporter)

Comment 10

6 months ago
Comment on attachment 9011898 [details] [diff] [review]
bug-1451715.patch

>--- a/browser/components/sessionstore/SessionStore.jsm
>+++ b/browser/components/sessionstore/SessionStore.jsm

>+setCustomGlobalValue(aKey, aStringValue) {
>+    SessionStoreInternal.setCustomGlobalValue(aKey, aStringValue);
>+  },
>+
>+deleteCustomGlobalValue(aKey) {
>+    SessionStoreInternal.deleteCustomGlobalValue(aKey);
>   },

nit: indentation is off at the method headers
Flags: needinfo?(dao+bmo)
Attachment #9011898 - Flags: review+
(Assignee)

Comment 11

6 months ago
That nit is legit. Patch updated and re–submitted. Thanks!
Attachment #9011898 - Attachment is obsolete: true
Flags: needinfo?(dao+bmo)
Attachment #9012156 - Flags: review+
(Reporter)

Updated

6 months ago
status-firefox61: affected → ---
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 13

6 months ago
Comment on attachment 9012156 [details] [diff] [review]
bug-1451715.patch

Looks good, thanks!
Attachment #9012156 - Flags: review?(dao+bmo) → review+
(Reporter)

Updated

6 months ago
Keywords: checkin-needed

Comment 14

6 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51cd5f6c6fa3
Rename {get,set,delete}{Window,Global}Value to match {get,set,delete}CustomTabValue. r=dao
Keywords: checkin-needed

Comment 15

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/51cd5f6c6fa3
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.