--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -236,17 +236,17 @@ var StarUI = {
this._autoCloseTimerEnabled = true;
}
break;
}
},
_overlayLoaded: false,
_overlayLoading: false,
- async showEditBookmarkPopup(aNode, aAnchorElement, aPosition, aIsNewBookmark) {
+ async showEditBookmarkPopup(aNode, aAnchorElement, aPosition, aIsNewBookmark, aUrl) {
// Slow double-clicks (not true double-clicks) shouldn't
// cause the panel to flicker.
if (this.panel.state == "showing" ||
this.panel.state == "open") {
return;
}
this._isNewBookmark = aIsNewBookmark;
@@ -261,39 +261,39 @@ var StarUI = {
}
// Performance: load the overlay the first time the panel is opened
// (see bug 392443).
if (this._overlayLoading)
return;
if (this._overlayLoaded) {
- await this._doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition);
+ await this._doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition, aUrl);
return;
}
this._overlayLoading = true;
document.loadOverlay(
"chrome://browser/content/places/editBookmarkOverlay.xul",
(aSubject, aTopic, aData) => {
// Move the header (star, title, button) into the grid,
// so that it aligns nicely with the other items (bug 484022).
let header = this._element("editBookmarkPanelHeader");
let rows = this._element("editBookmarkPanelGrid").lastChild;
rows.insertBefore(header, rows.firstChild);
header.hidden = false;
this._overlayLoading = false;
this._overlayLoaded = true;
- this._doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition);
+ this._doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition, aUrl);
}
);
},
- async _doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition) {
+ async _doShowEditBookmarkPanel(aNode, aAnchorElement, aPosition, aUrl) {
if (this.panel.state != "closed")
return;
this._blockCommands(); // un-done in the popuphidden handler
this._element("editBookmarkPanelTitle").value =
this._isNewBookmark ?
gNavigatorBundle.getString("editBookmarkPanel.pageBookmarkedTitle") :
@@ -303,17 +303,17 @@ var StarUI = {
this._element("editBookmarkPanelDescription").textContent = "";
this._element("editBookmarkPanelBottomButtons").hidden = false;
this._element("editBookmarkPanelContent").hidden = false;
// The label of the remove button differs if the URI is bookmarked
// multiple times.
this._itemGuids = [];
- await PlacesUtils.bookmarks.fetch({url: gBrowser.currentURI},
+ await PlacesUtils.bookmarks.fetch({url: aUrl},
bookmark => this._itemGuids.push(bookmark.guid));
if (!PlacesUIUtils.useAsyncTransactions) {
this._itemIdsMap = await PlacesUtils.promiseManyItemIds(this._itemGuids);
}
let forms = gNavigatorBundle.getString("editBookmark.removeBookmarks.label");
let bookmarksCount = this._itemGuids.length;
let label = PluralForm.get(bookmarksCount, forms)
@@ -436,40 +436,48 @@ var PlacesCommandHook = {
*
* @param aBrowser
* a <browser> element.
* @param [optional] aParent
* The folder in which to create a new bookmark if the page loaded in
* aBrowser isn't bookmarked yet, defaults to the unfiled root.
* @param [optional] aShowEditUI
* whether or not to show the edit-bookmark UI for the bookmark item
+ * @param [optional] aUrl
+ * Option to provide a URL to bookmark rather than the current page
+ * @param [optional] aTitle
+ * Option to provide a title for a bookmark to use rather than the
+ * getting the current page's title
*/
- async bookmarkPage(aBrowser, aParent, aShowEditUI) {
+ async bookmarkPage(aBrowser, aParent, aShowEditUI, aUrl = null, aTitle = null) {
if (PlacesUIUtils.useAsyncTransactions) {
- await this._bookmarkPagePT(aBrowser, aParent, aShowEditUI);
+ await this._bookmarkPagePT(aBrowser, aParent, aShowEditUI, aUrl, aTitle);
return;
}
- var uri = aBrowser.currentURI;
+ // If aUrl is provided, we want to bookmark that url rather than the
+ // the current page
+ var uri = aUrl ? Services.io.newURI(aUrl) : aBrowser.currentURI;
var itemId = PlacesUtils.getMostRecentBookmarkForURI(uri);
let isNewBookmark = itemId == -1;
if (isNewBookmark) {
// Bug 1148838 - Make this code work for full page plugins.
var title;
var description;
var charset;
- let docInfo = await this._getPageDetails(aBrowser);
+ let docInfo = aUrl ? {} : await this._getPageDetails(aBrowser);
try {
- title = docInfo.isErrorPage ? PlacesUtils.history.getPageTitle(uri)
- : aBrowser.contentTitle;
- title = title || uri.displaySpec;
+ title = aTitle ||
+ (docInfo.isErrorPage ? PlacesUtils.history.getPageTitle(uri)
+ : aBrowser.contentTitle) ||
+ uri.displaySpec;
description = docInfo.description;
- charset = aBrowser.characterSet;
+ charset = aUrl ? null : aBrowser.characterSet;
} catch (e) { }
if (aShowEditUI) {
// If we bookmark the page here but open right into a cancelable
// state (i.e. new bookmark in Library), start batching here so
// all of the actions can be undone in a single undo step.
StarUI.beginBatch();
}
@@ -495,58 +503,60 @@ var PlacesCommandHook = {
return;
// Try to dock the panel to:
// 1. the bookmarks menu button
// 2. the identity icon
// 3. the content area
if (BookmarkingUI.anchor && isVisible(BookmarkingUI.anchor)) {
await StarUI.showEditBookmarkPopup(itemId, BookmarkingUI.anchor,
- "bottomcenter topright", isNewBookmark);
+ "bottomcenter topright", isNewBookmark, uri);
return;
}
let identityIcon = document.getElementById("identity-icon");
if (isVisible(identityIcon)) {
await StarUI.showEditBookmarkPopup(itemId, identityIcon,
- "bottomcenter topright", isNewBookmark);
+ "bottomcenter topright", isNewBookmark, uri);
} else {
- await StarUI.showEditBookmarkPopup(itemId, aBrowser, "overlap", isNewBookmark);
+ await StarUI.showEditBookmarkPopup(itemId, aBrowser, "overlap", isNewBookmark, uri);
}
},
// TODO: Replace bookmarkPage code with this function once legacy
// transactions are removed.
- async _bookmarkPagePT(aBrowser, aParentId, aShowEditUI) {
- let url = new URL(aBrowser.currentURI.spec);
+ async _bookmarkPagePT(aBrowser, aParentId, aShowEditUI, aUrl, aTitle) {
+ // If aUrl is provided, we want to bookmark that url rather than the
+ // the current page
+ let url = aUrl ? new URL(aUrl) : new URL(aBrowser.currentURI.spec);
let info = await PlacesUtils.bookmarks.fetch({ url });
let isNewBookmark = !info;
if (!info) {
let parentGuid = aParentId !== undefined ?
await PlacesUtils.promiseItemGuid(aParentId) :
PlacesUtils.bookmarks.unfiledGuid;
info = { url, parentGuid };
// Bug 1148838 - Make this code work for full page plugins.
let description = null;
let charset = null;
- let docInfo = await this._getPageDetails(aBrowser);
+ let docInfo = aUrl ? {} : await this._getPageDetails(aBrowser);
try {
if (docInfo.isErrorPage) {
let entry = await PlacesUtils.history.fetch(aBrowser.currentURI);
if (entry) {
info.title = entry.title;
}
} else {
- info.title = aBrowser.contentTitle;
+ info.title = aTitle || aBrowser.contentTitle;
}
info.title = info.title || url.href;
description = docInfo.description;
- charset = aBrowser.characterSet;
+ charset = aUrl ? null : aBrowser.characterSet;
} catch (e) {
Components.utils.reportError(e);
}
if (aShowEditUI && isNewBookmark) {
// If we bookmark the page here but open right into a cancelable
// state (i.e. new bookmark in Library), start batching here so
// all of the actions can be undone in a single undo step.
@@ -575,26 +585,26 @@ var PlacesCommandHook = {
let node = await PlacesUIUtils.promiseNodeLikeFromFetchInfo(info);
// Try to dock the panel to:
// 1. the bookmarks menu button
// 2. the identity icon
// 3. the content area
if (BookmarkingUI.anchor && isVisible(BookmarkingUI.anchor)) {
await StarUI.showEditBookmarkPopup(node, BookmarkingUI.anchor,
- "bottomcenter topright", isNewBookmark);
+ "bottomcenter topright", isNewBookmark, url);
return;
}
let identityIcon = document.getElementById("identity-icon");
if (isVisible(identityIcon)) {
await StarUI.showEditBookmarkPopup(node, identityIcon,
- "bottomcenter topright", isNewBookmark);
+ "bottomcenter topright", isNewBookmark, url);
} else {
- await StarUI.showEditBookmarkPopup(node, aBrowser, "overlap", isNewBookmark);
+ await StarUI.showEditBookmarkPopup(node, aBrowser, "overlap", isNewBookmark, url);
}
},
_getPageDetails(browser) {
return new Promise(resolve => {
let mm = browser.messageManager;
mm.addMessageListener("Bookmarks:GetPageDetails:Result", function listener(msg) {
mm.removeMessageListener("Bookmarks:GetPageDetails:Result", listener);
--- a/browser/base/content/test/general/browser_bookmark_titles.js
+++ b/browser/base/content/test/general/browser_bookmark_titles.js
@@ -14,17 +14,17 @@ var tests = [
// about:neterror
["data:application/vnd.mozilla.xul+xml,",
"data:application/vnd.mozilla.xul+xml,"],
// about:certerror
["https://untrusted.example.com/somepage.html",
"https://untrusted.example.com/somepage.html"]
];
-add_task(async function() {
+add_task(async function check_default_bookmark_title() {
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
let browser = tab.linkedBrowser;
// Test that a bookmark of each URI gets the corresponding default title.
for (let i = 0; i < tests.length; ++i) {
let [url, title] = tests[i];
// We use promisePageLoaded rather than BrowserTestUtils.browserLoaded - see
@@ -65,16 +65,57 @@ add_task(async function() {
Assert.equal(content.document.documentURI.substring(0, 14), "about:neterror",
"Offline mode successfully simulated network outage.");
});
await checkBookmark(url, title);
await BrowserTestUtils.removeTab(tab);
});
+add_task(async function check_override_bookmark_title() {
+ let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+ let browser = tab.linkedBrowser;
+ let [url, default_title] = tests[1];
+
+ // We use promisePageLoaded rather than BrowserTestUtils.browserLoaded - see
+ // note on function definition below.
+ let promiseLoaded = promisePageLoaded(browser);
+ BrowserTestUtils.loadURI(browser, url);
+
+ await promiseLoaded;
+
+ // Test that a bookmark of this URI gets the correct title if we provide one
+ await checkBookmarkedPageTitle(url, default_title, "An overridden title");
+
+ await BrowserTestUtils.removeTab(tab);
+});
+
+// Bookmark a page and confirm that the new bookmark has the expected title.
+// (Then delete the bookmark.)
+async function checkBookmarkedPageTitle(url, default_title, overridden_title) {
+ let promiseBookmark = PlacesTestUtils.waitForNotification("onItemAdded",
+ (id, parentId, index, type, itemUrl) => itemUrl.equals(Services.io.newURI(url)));
+
+ // Here we test that if we provide a url and a title to bookmark, it will use the
+ // title provided rather than the one provided by the current page
+ PlacesCommandHook.bookmarkPage(gBrowser.selectedBrowser, undefined, false, url, overridden_title);
+ await promiseBookmark;
+
+ let bookmark = await PlacesUtils.bookmarks.fetch({url});
+
+ Assert.ok(bookmark, "Found the expected bookmark");
+ Assert.equal(bookmark.title, overridden_title, "Bookmark got a good overridden title.");
+ Assert.equal(default_title, gBrowser.selectedBrowser.contentTitle,
+ "Sanity check that the content is providing us with the correct title");
+ Assert.notEqual(bookmark.title, default_title,
+ "Make sure we picked the overridden one and not the default one.");
+
+ await PlacesUtils.bookmarks.remove(bookmark);
+}
+
// Bookmark the current page and confirm that the new bookmark has the expected
// title. (Then delete the bookmark.)
async function checkBookmark(url, expected_title) {
Assert.equal(gBrowser.selectedBrowser.currentURI.spec, url,
"Trying to bookmark the expected uri");
let promiseBookmark = PlacesTestUtils.waitForNotification("onItemAdded",
(id, parentId, index, type, itemUrl) => itemUrl.equals(gBrowser.selectedBrowser.currentURI));
--- a/browser/extensions/activity-stream/data/content/activity-stream.bundle.js
+++ b/browser/extensions/activity-stream/data/content/activity-stream.bundle.js
@@ -2577,17 +2577,17 @@ module.exports = {
}),
userEvent: "BOOKMARK_DELETE"
}),
AddBookmark: site => ({
id: "menu_action_bookmark",
icon: "bookmark",
action: ac.SendToMain({
type: at.BOOKMARK_URL,
- data: site.url
+ data: {url: site.url, title: site.title}
}),
userEvent: "BOOKMARK_ADD"
}),
OpenInNewWindow: site => ({
id: "menu_action_open_new_window",
icon: "new-window",
action: ac.SendToMain({
type: at.OPEN_NEW_WINDOW,
@@ -2727,9 +2727,9 @@ ReactDOM.render(React.createElement(
Provider,
{ store: store },
React.createElement(Base, null)
), document.getElementById("root"));
addSnippetsSubscriber(store);
/***/ })
-/******/ ]);
\ No newline at end of file
+/******/ ]);
--- a/browser/extensions/activity-stream/lib/PlacesFeed.jsm
+++ b/browser/extensions/activity-stream/lib/PlacesFeed.jsm
@@ -202,17 +202,17 @@ class PlacesFeed {
break;
case at.UNINIT:
this.removeObservers();
break;
case at.BLOCK_URL:
NewTabUtils.activityStreamLinks.blockURL({url: action.data});
break;
case at.BOOKMARK_URL:
- NewTabUtils.activityStreamLinks.addBookmark(action.data);
+ NewTabUtils.activityStreamLinks.addBookmark(action.data, action._target.browser);
break;
case at.DELETE_BOOKMARK_BY_ID:
NewTabUtils.activityStreamLinks.deleteBookmark(action.data);
break;
case at.DELETE_HISTORY_URL:
NewTabUtils.activityStreamLinks.deleteHistoryEntry(action.data);
break;
case at.OPEN_NEW_WINDOW:
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -1125,28 +1125,35 @@ var ActivityStreamLinks = {
BlockedLinks.block(aLink);
},
onLinkBlocked(aLink) {
Services.obs.notifyObservers(null, "newtab-linkBlocked", aLink.url);
},
/**
- * Adds a bookmark
+ * Adds a bookmark and opens up the Bookmark Dialog to show feedback that
+ * the bookmarking action has been successful
*
- * @param {String} aUrl
- * The url to bookmark
+ * @param {Object} aData
+ * aData.url The url to bookmark
+ * aData.title The title of the page to bookmark
+ * @param {Browser} aBrowser
+ * a <browser> element
*
* @returns {Promise} Returns a promise set to an object representing the bookmark
*/
- addBookmark(aUrl) {
- return PlacesUtils.bookmarks.insert({
- url: aUrl,
- parentGuid: PlacesUtils.bookmarks.unfiledGuid
- });
+ addBookmark(aData, aBrowser) {
+ const {url, title} = aData;
+ return aBrowser.ownerGlobal.PlacesCommandHook.bookmarkPage(
+ aBrowser,
+ undefined,
+ true,
+ url,
+ title);
},
/**
* Removes a bookmark
*
* @param {String} aBookmarkGuid
* The bookmark guid associated with the bookmark to remove
*
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -550,48 +550,16 @@ add_task(async function activitySteamPro
let deleted = await provider.deleteHistoryEntry("https://mozilla2.com/1");
Assert.equal(deleted, true, "link is deleted");
// ensure that there's only one link left
size = await NewTabUtils.activityStreamProvider.getHistorySize();
Assert.equal(size, 1, "expected history size");
});
-add_task(async function activityStream_addBookmark() {
- await setUpActivityStreamTest();
-
- let provider = NewTabUtils.activityStreamLinks;
- let bookmarks = [
- "https://mozilla1.com/0",
- "https://mozilla1.com/1"
- ];
-
- let bookmarksSize = await NewTabUtils.activityStreamProvider.getBookmarksSize();
- Assert.equal(bookmarksSize, 0, "empty bookmarks yields 0 size");
-
- for (let url of bookmarks) {
- await provider.addBookmark(url);
- }
- bookmarksSize = await NewTabUtils.activityStreamProvider.getBookmarksSize();
- Assert.equal(bookmarksSize, 2, "size 2 for 2 bookmarks added");
-});
-
-add_task(async function activityStream_getBookmark() {
- await setUpActivityStreamTest();
-
- let provider = NewTabUtils.activityStreamLinks;
- let bookmark = await provider.addBookmark("https://mozilla1.com/0");
-
- let result = await NewTabUtils.activityStreamProvider.getBookmark(bookmark.guid);
- Assert.equal(result.bookmarkGuid, bookmark.guid, "got the correct bookmark guid");
- Assert.equal(result.bookmarkTitle, bookmark.title, "got the correct bookmark title");
- Assert.equal(result.lastModified, bookmark.lastModified.getTime(), "got the correct bookmark time");
- Assert.equal(result.url, bookmark.url.href, "got the correct bookmark url");
-});
-
add_task(async function activityStream_deleteBookmark() {
await setUpActivityStreamTest();
let provider = NewTabUtils.activityStreamLinks;
let bookmarks = [
{url: "https://mozilla1.com/0", parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK},
{url: "https://mozilla1.com/1", parentGuid: PlacesUtils.bookmarks.unfiledGuid, type: PlacesUtils.bookmarks.TYPE_BOOKMARK}
];