Closed
Bug 1208874
Opened 9 years ago
Closed 9 years ago
localStorage in background script seems to be temporary
Categories
(WebExtensions :: Untriaged, defect, P1)
WebExtensions
Untriaged
Tracking
(firefox45 fixed)
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: evilpie, Assigned: billm)
References
Details
(Whiteboard: [storage])
Attachments
(1 file)
9.03 KB,
patch
|
kmag
:
review+
|
Details | Diff | Splinter Review |
It looks like RES uses localStorage in its background script, which also seems to be reset everytime. I wonder why they don't use chrome.storage.
Assignee | ||
Comment 1•9 years ago
|
||
This is caused by the fact that we regenerate the UUID that goes in moz-extension://<uuid>/ every startup. We should probably generate it once per install or something like that. Another option would be to hash the telemetry client ID together with the extension ID.
Updated•9 years ago
|
Whiteboard: [storage]
Comment 2•9 years ago
|
||
(In reply to Tom Schuster [:evilpie] from comment #0) > I wonder why they don't use chrome.storage. Not sure about RES, but a strong case for localStorage vs chrome.storage is synchronous access. Is there any sure-fire mean for an extension to read its persistent configuration *before* anything "interesting" happens in content-land? This is especially painful for content blocking extensions of course: as far as I know while this can be worked around in background pages (via localStorage), but the situation is far worse in content scripts, which in Chromium cannot send synchronous messages to retrieve their settings (one of the reasons there's no NoScript for Chromium). Bill, is there anything I'm missing? Have we got something already, or a plan for early configuration retrieval in WebExtensions?
Flags: needinfo?(wmccloskey)
Assignee | ||
Comment 3•9 years ago
|
||
I think we just need to fix this bug. Would that work for you?
Flags: needinfo?(wmccloskey)
Updated•9 years ago
|
Flags: blocking-webextensions+
Assignee | ||
Comment 5•9 years ago
|
||
This takes a pretty direct approach. I used a pref to store the extension ID -> UUID mapping. One downside is that nothing ever gets removed from this pref. That can be fixed in bug 1213990.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8685683 -
Flags: review?(kmaglione+bmo)
Comment 6•9 years ago
|
||
Comment on attachment 8685683 [details] [diff] [review] patch Review of attachment 8685683 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/extensions/Extension.jsm @@ +677,5 @@ > + let uuid = uuidGenerator.generateUUID().number; > + uuid = uuid.slice(1, -1); // Strip of { and } off the UUID. > + > + map[id] = uuid; > + Preferences.set(PREF_NAME, JSON.stringify(map)); We should really probably clear this on uninstall. But that would leave orphaned localStorage data, and I think there's still some question about whether we want to clear add-on data on uninstall anyway, so maybe just add a comment and a note to the Chrome incompatibilities docs?
Attachment #8685683 -
Flags: review?(kmaglione+bmo) → review+
Comment 8•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9e61ae324d13
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Updated•9 years ago
|
Iteration: --- → 45.1 - Nov 16
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•