Closed Bug 1233907 Opened 7 years ago Closed 6 years ago

bookmark import needs to use the default user context origin attributes

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: huseby, Assigned: huseby)

References

(Blocks 1 open bug)

Details

(Whiteboard: [userContextId][OA])

Attachments

(1 file, 2 obsolete files)

In the toolkit/components/places/BookmarkJSONUtils.jsm file there is a call to createCodebasePrincipal that needs to use the default global user context since we aren't doing any kind of user context isolation of bookmarks yet.

> toolkit/components/places/BookmarkJSONUtils.jsm
>   line 210
>       let channel = NetUtil.newChannel({
>         uri,
>         loadingPrincipal: Services.scriptSecurityManager.createCodebasePrincipal(uri, {}),
>         contentPolicyType: Ci.nsIContentPolicy.TYPE_INTERNAL_XMLHTTPREQUEST
>       });
Component: Bookmarks & History → DOM
Product: Firefox → Core
Component: DOM → Bookmarks & History
Product: Core → Firefox
Whiteboard: [userContextId]
Attached patch Bug_1233907.patch (obsolete) — Splinter Review
Attached patch Bug_1233907.patch (obsolete) — Splinter Review
updated for new API.
Attachment #8717680 - Attachment is obsolete: true
Attachment #8722055 - Flags: review?(jonas)
Comment on attachment 8722055 [details] [diff] [review]
Bug_1233907.patch

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

::: toolkit/components/places/BookmarkJSONUtils.jsm
@@ +207,5 @@
>        };
>  
>        let uri = NetUtil.newURI(spec);
> +      let attrs ChromeUtils.createOriginAttributesFromOrigin(spec);
> +      attrs.userContextId = 0;

Since this is run by chrome code, i think you can simply assert that the userContextId is 0 here.

Also, you're missing a '=' after 'attrs' on the first line.

And, if 'spec' is a valid URI, then the ChromeUtils.createOriginAttributesFromOrigin call here is effectively the same as ChromeUtils.createOriginAttributes({}), right?
Attachment #8722055 - Flags: review?(jonas) → review-
switched the patch to just an assert.
Attachment #8722055 - Attachment is obsolete: true
Attachment #8726502 - Flags: review?(jonas)
oooh, look at all of the red.  try one more time just to see: https://treeherder.mozilla.org/#/jobs?repo=try&revision=975cce4ce590
Comment on attachment 8726502 [details] [diff] [review]
Bug_1233907.patch

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

r=me assuming that NS_ASSERT is in fact defined here.
Attachment #8726502 - Flags: review?(jonas) → review+
I just rebased and notcied that the code has been refactored to support loading bookmarks with the system principal: http://mzl.la/1Mqr3zQ

This patch is no longer needed AFAICT.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Whiteboard: [userContextId] → [userContextId][OA]
You need to log in before you can comment on or make changes to this bug.