diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 8fa1f7bad0a5..8227f30fd636 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1881,5 +1881,6 @@ pref("reader.parse-on-load.enabled", false); // Disable ReadingList browser UI by default. pref("browser.readinglist.enabled", false); +pref("browser.readinglist.sidebarEverOpened", false); // Enable the readinglist engine by default. pref("readinglist.scheduler.enabled", true); diff --git a/browser/base/content/browser-readinglist.js b/browser/base/content/browser-readinglist.js index b3cda23a73ce..4c4d5952f65d 100644 --- a/browser/base/content/browser-readinglist.js +++ b/browser/base/content/browser-readinglist.js @@ -178,7 +178,7 @@ let ReadingListUI = { }); target.insertBefore(menuitem, insertPoint); - }); + }, {sort: "addedOn", descending: true}); if (!hasItems) { let menuitem = document.createElement("menuitem"); @@ -242,12 +242,20 @@ let ReadingListUI = { uri = null; } + let msg = {topic: "UpdateActiveItem", url: null}; if (!uri) { this.toolbarButton.setAttribute("hidden", true); + if (this.isSidebarOpen) + document.getElementById("sidebar").contentWindow.postMessage(msg, "*"); return; } - let isInList = yield ReadingList.containsURL(uri); + let isInList = yield ReadingList.hasItemForURL(uri); + if (this.isSidebarOpen) { + if (isInList) + msg.url = typeof uri == "string" ? uri : uri.spec; + document.getElementById("sidebar").contentWindow.postMessage(msg, "*"); + } this.setToolbarButtonState(isInList); }), @@ -284,7 +292,7 @@ let ReadingListUI = { if (!uri) return; - let item = yield ReadingList.getItemForURL(uri); + let item = yield ReadingList.itemForURL(uri); if (item) { yield item.delete(); } else { @@ -315,8 +323,15 @@ let ReadingListUI = { * @param {ReadingListItem} item - Item added. */ onItemAdded(item) { + if (!Services.prefs.getBoolPref("browser.readinglist.sidebarEverOpened")) { + SidebarUI.show("readingListSidebar"); + } if (this.isItemForCurrentBrowser(item)) { this.setToolbarButtonState(true); + if (this.isSidebarOpen) { + let msg = {topic: "UpdateActiveItem", url: item.url}; + document.getElementById("sidebar").contentWindow.postMessage(msg, "*"); + } } }, diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css index f52f313c52fb..a8d59da11450 100644 --- a/browser/base/content/browser.css +++ b/browser/base/content/browser.css @@ -915,7 +915,6 @@ chatbox { chatbox[large="true"] { width: 300px; - heigth: 272px; } chatbox[minimized="true"] { diff --git a/browser/base/content/newtab/grid.js b/browser/base/content/newtab/grid.js index bdf9ab54aa92..f8acbab375fe 100644 --- a/browser/base/content/newtab/grid.js +++ b/browser/base/content/newtab/grid.js @@ -168,7 +168,8 @@ let gGrid = { ' class="newtab-control newtab-control-pin"/>' + '' + - '' + newTabString("sponsored.button") + ''; + '' + newTabString("sponsored.button") + '' + + ''; this._siteFragment = document.createDocumentFragment(); this._siteFragment.appendChild(site); @@ -189,7 +190,8 @@ let gGrid = { // Save the cell's computed height/width including margin and border if (this._cellMargin === undefined) { let refCell = document.querySelector(".newtab-cell"); - this._cellMargin = parseFloat(getComputedStyle(refCell).marginTop) * 2; + this._cellMargin = parseFloat(getComputedStyle(refCell).marginTop) + + parseFloat(getComputedStyle(refCell).marginBottom); this._cellHeight = refCell.offsetHeight + this._cellMargin; this._cellWidth = refCell.offsetWidth + this._cellMargin; } diff --git a/browser/base/content/newtab/newTab.css b/browser/base/content/newtab/newTab.css index cef72f852620..bd0335a2eaab 100644 --- a/browser/base/content/newtab/newTab.css +++ b/browser/base/content/newtab/newTab.css @@ -159,7 +159,7 @@ input[type=button] { .newtab-cell { display: -moz-box; height: 180px; - margin: 20px 10px; + margin: 20px 10px 85px; width: 290px; } @@ -193,20 +193,45 @@ input[type=button] { /* TITLES */ .newtab-sponsored, -.newtab-title { - bottom: -26px; +.newtab-title, +.newtab-suggested { overflow: hidden; position: absolute; right: 0; text-align: center; +} + +.newtab-sponsored, +.newtab-title { + bottom: -26px; white-space: nowrap; + text-overflow: ellipsis; + font-size: 13px; +} + +.newtab-suggested { + border: 1px solid #dcdcdc; + border-radius: 2px; + cursor: pointer; + font-size: 12px; + height: 17px; + line-height: 17px; + margin-bottom: -1px; + padding: 2px 8px; + display: none; + margin-left: auto; + margin-right: auto; + left: 0; + top: 215px; +} + +.newtab-suggested-bounds { + max-height: 51px; /* 51 / 17 = 3 lines maximum */ } .newtab-title { - font-size: 13px; left: 0; padding-top: 14px; - text-overflow: ellipsis; } .newtab-sponsored { @@ -237,6 +262,10 @@ input[type=button] { display: block; } +.newtab-site[type=related] .newtab-suggested { + display: table; +} + .sponsored-explain, .sponsored-explain a { color: white; @@ -462,7 +491,7 @@ input[type=button] { .newtab-customize-panel-item, .newtab-search-panel-engine, #newtab-search-manage { - padding: 4px 24px; + padding: 10px 10px 10px 25px; } .newtab-customize-panel-item:not(:last-child), @@ -484,6 +513,10 @@ input[type=button] { margin: 0; } +.newtab-customize-panel-item:not([selected]) { + color: #919191; +} + .newtab-customize-panel-item[selected], .newtab-search-panel-engine[selected] { background: url("chrome://global/skin/menu/shared-menu-check.png") center left 4px no-repeat transparent; diff --git a/browser/base/content/newtab/newTab.xul b/browser/base/content/newtab/newTab.xul index ebde760b1ec4..bc6a0c2002fe 100644 --- a/browser/base/content/newtab/newTab.xul +++ b/browser/base/content/newtab/newTab.xul @@ -37,13 +37,13 @@ diff --git a/browser/base/content/newtab/sites.js b/browser/base/content/newtab/sites.js index 9ee9f62d9979..526c57c8af18 100644 --- a/browser/base/content/newtab/sites.js +++ b/browser/base/content/newtab/sites.js @@ -131,6 +131,12 @@ Site.prototype = { this._querySelector(".newtab-title").textContent = title; this.node.setAttribute("type", this.link.type); + if (this.link.targetedSite) { + let targetedSite = ` ${this.link.targetedSite} `; + this._querySelector(".newtab-suggested").innerHTML = + `
${newTabString("suggested.button", [targetedSite])}
`; + } + if (this.isPinned()) this._updateAttributes(true); // Capture the page if the thumbnail is missing, which will cause page.js @@ -177,6 +183,15 @@ Site.prototype = { } }, + _ignoreHoverEvents: function(element) { + element.addEventListener("mouseover", () => { + this.cell.node.setAttribute("ignorehover", "true"); + }); + element.addEventListener("mouseout", () => { + this.cell.node.removeAttribute("ignorehover"); + }); + }, + /** * Adds event handlers for the site and its buttons. */ @@ -186,14 +201,12 @@ Site.prototype = { this._node.addEventListener("dragend", this, false); this._node.addEventListener("mouseover", this, false); - // Specially treat the sponsored icon to prevent regular hover effects + // Specially treat the sponsored icon & suggested explanation + // text to prevent regular hover effects let sponsored = this._querySelector(".newtab-sponsored"); - sponsored.addEventListener("mouseover", () => { - this.cell.node.setAttribute("ignorehover", "true"); - }); - sponsored.addEventListener("mouseout", () => { - this.cell.node.removeAttribute("ignorehover"); - }); + let suggested = this._querySelector(".newtab-suggested"); + this._ignoreHoverEvents(sponsored); + this._ignoreHoverEvents(suggested); }, /** @@ -268,6 +281,12 @@ Site.prototype = { } // Only handle primary clicks for the remaining targets else if (button == 0) { + if (target.parentElement.classList.contains("newtab-suggested") || + target.classList.contains("newtab-suggested")) { + // Suggested explanation text should do nothing when clicked and + // the link in the suggested explanation should act as default. + return; + } aEvent.preventDefault(); if (target.classList.contains("newtab-control-block")) { this.block(); diff --git a/browser/base/content/test/newtab/browser_newtab_drag_drop.js b/browser/base/content/test/newtab/browser_newtab_drag_drop.js index b32005e119a6..76cc216b8ea0 100644 --- a/browser/base/content/test/newtab/browser_newtab_drag_drop.js +++ b/browser/base/content/test/newtab/browser_newtab_drag_drop.js @@ -60,8 +60,8 @@ function runTests() { yield addNewTabPageTab(); checkGrid("0,1,2,3,4,5,6,7p,8p"); - yield simulateDrop(2, 8); - checkGrid("0,1,3,4,5,6,7p,8p,2p"); + yield simulateDrop(2, 5); + checkGrid("0,1,3,4,5,2p,6,7p,8p"); // make sure that pinned sites are re-positioned correctly yield setLinks("0,1,2,3,4,5,6,7,8"); diff --git a/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js b/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js index f7b9cf8fb4dd..745ba939f263 100644 --- a/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js +++ b/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js @@ -35,8 +35,8 @@ function runTests() { // force the grid to be small enough that a pinned cell could be pushed out Services.prefs.setIntPref(PREF_NEWTAB_COLUMNS, 3); - yield simulateExternalDrop(7); - checkGrid("0,1,2,3,4,5,7p,99p,8p"); + yield simulateExternalDrop(5); + checkGrid("0,1,2,3,4,99p,5,7p,8p"); // drag a new site beneath a pinned cell and make sure the pinned cell is // not moved @@ -46,8 +46,8 @@ function runTests() { yield addNewTabPageTab(); checkGrid("0,1,2,3,4,5,6,7,8p"); - yield simulateExternalDrop(7); - checkGrid("0,1,2,3,4,5,6,99p,8p"); + yield simulateExternalDrop(5); + checkGrid("0,1,2,3,4,99p,5,6,8p"); // drag a new site onto a block of pinned sites and make sure they're shifted // around accordingly diff --git a/browser/base/content/test/newtab/browser_newtab_enhanced.js b/browser/base/content/test/newtab/browser_newtab_enhanced.js index c45f155811ca..d2bbb5a00c55 100644 --- a/browser/base/content/test/newtab/browser_newtab_enhanced.js +++ b/browser/base/content/test/newtab/browser_newtab_enhanced.js @@ -40,21 +40,21 @@ function runTests() { yield addNewTabPageTab(); yield customizeNewTabPage("classic"); let {type, enhanced, title} = getData(0); - is(type, "organic", "directory link is organic"); - isnot(enhanced, "", "directory link has enhanced image"); - is(title, "title"); + isnot(type, "enhanced", "history link is not enhanced"); + is(enhanced, "", "history link has no enhanced image"); + is(title, "site#-1"); - is(getData(1), null, "history link pushed out by directory link"); + is(getData(1), null, "there is only one link and it's a history link"); // Test with enhanced = true yield addNewTabPageTab(); yield customizeNewTabPage("enhanced"); ({type, enhanced, title} = getData(0)); - is(type, "organic", "directory link is still organic"); - isnot(enhanced, "", "directory link still has enhanced image"); + is(type, "organic", "directory link is organic"); + isnot(enhanced, "", "directory link has enhanced image"); is(title, "title"); - is(getData(1), null, "history link still pushed out by directory link"); + is(getData(1), null, "history link pushed out by directory link"); // Test with a pinned link setPinnedLinks("-1"); diff --git a/browser/components/preferences/in-content/content.js b/browser/components/preferences/in-content/content.js index 8b2d23861c36..f5c9ed80cb76 100644 --- a/browser/components/preferences/in-content/content.js +++ b/browser/components/preferences/in-content/content.js @@ -26,7 +26,7 @@ var gContentPane = { row.removeAttribute("hidden"); } - setEventListener("font.language.group", "blur", + setEventListener("font.language.group", "change", gContentPane._rebuildFonts); setEventListener("popupPolicyButton", "command", gContentPane.showPopupExceptions); diff --git a/browser/components/readinglist/ReadingList.jsm b/browser/components/readinglist/ReadingList.jsm index bc97fc2c8415..cd4c847681f4 100644 --- a/browser/components/readinglist/ReadingList.jsm +++ b/browser/components/readinglist/ReadingList.jsm @@ -33,8 +33,10 @@ XPCOMUtils.defineLazyModuleGetter(this, "SQLiteStore", let log = Log.repository.getLogger("readinglist.api"); -// Names of basic properties on ReadingListItem. -const ITEM_BASIC_PROPERTY_NAMES = ` +// Each ReadingListItem has a _record property, an object containing the raw +// data from the server and local store. These are the names of the properties +// in that object. +const ITEM_RECORD_PROPERTIES = ` guid lastModified url @@ -71,7 +73,7 @@ const ITEM_BASIC_PROPERTY_NAMES = ` * control the items that the method acts on. * * Each options object is a simple object with properties whose names are drawn - * from ITEM_BASIC_PROPERTY_NAMES. For an item to match an options object, the + * from ITEM_RECORD_PROPERTIES. For an item to match an options object, the * properties of the item must match all the properties in the object. For * example, an object { guid: "123" } matches any item whose GUID is 123. An * object { guid: "123", title: "foo" } matches any item whose GUID is 123 *and* @@ -89,7 +91,7 @@ const ITEM_BASIC_PROPERTY_NAMES = ` * options object { guid: ["123", "456"] } matches any item whose GUID is either * 123 *or* 456. * - * In addition to properties with names from ITEM_BASIC_PROPERTY_NAMES, options + * In addition to properties with names from ITEM_RECORD_PROPERTIES, options * objects can also have the following special properties: * * * sort: The name of a property to sort on. @@ -109,14 +111,14 @@ const ITEM_BASIC_PROPERTY_NAMES = ` */ function ReadingListImpl(store) { this._store = store; - this._itemsByURL = new Map(); + this._itemsByNormalizedURL = new Map(); this._iterators = new Set(); this._listeners = new Set(); } ReadingListImpl.prototype = { - ItemBasicPropertyNames: ITEM_BASIC_PROPERTY_NAMES, + ItemRecordProperties: ITEM_RECORD_PROPERTIES, /** * Yields the number of items in the list. @@ -137,20 +139,20 @@ ReadingListImpl.prototype = { * @returns {Promise} Promise that is fulfilled with a boolean indicating * whether the URL is in the list or not. */ - containsURL: Task.async(function* (url) { + hasItemForURL: Task.async(function* (url) { url = normalizeURI(url).spec; // This is used on every tab switch and page load of the current tab, so we // want it to be quick and avoid a DB query whenever possible. // First check if any cached items have a direct match. - if (this._itemsByURL.has(url)) { + if (this._itemsByNormalizedURL.has(url)) { return true; } // Then check if any cached items may have a different resolved URL // that matches. - for (let itemWeakRef of this._itemsByURL.values()) { + for (let itemWeakRef of this._itemsByNormalizedURL.values()) { let item = itemWeakRef.get(); if (item && item.resolvedURL == url) { return true; @@ -177,10 +179,10 @@ ReadingListImpl.prototype = { */ forEachItem: Task.async(function* (callback, ...optsList) { let promiseChain = Promise.resolve(); - yield this._store.forEachItem(obj => { + yield this._store.forEachItem(record => { promiseChain = promiseChain.then(() => { return new Promise((resolve, reject) => { - let promise = callback(this._itemFromObject(obj)); + let promise = callback(this._itemFromRecord(record)); if (promise instanceof Promise) { return promise.then(resolve, reject); } @@ -210,23 +212,26 @@ ReadingListImpl.prototype = { * Adds an item to the list that isn't already present. * * The given object represents a new item, and the properties of the object - * are those in ITEM_BASIC_PROPERTY_NAMES. It may have as few or as many + * are those in ITEM_RECORD_PROPERTIES. It may have as few or as many * properties that you want to set, but it must have a `url` property. * * It's an error to call this with an object whose `url` or `guid` properties * are the same as those of items that are already present in the list. The * returned promise is rejected in that case. * - * @param obj A simple object representing an item. + * @param record A simple object representing an item. * @return Promise Resolved with the new item when the list * is updated. Rejected with an Error on error. */ - addItem: Task.async(function* (obj) { - obj = stripNonItemProperties(obj); - normalizeReadingListProperties(obj); - yield this._store.addItem(obj); + addItem: Task.async(function* (record) { + record = normalizeRecord(record); + record.addedOn = Date.now(); + if (Services.prefs.prefHasUserValue("services.sync.client.name")) { + record.addedBy = Services.prefs.getCharPref("services.sync.client.name"); + } + yield this._store.addItem(record); this._invalidateIterators(); - let item = this._itemFromObject(obj); + let item = this._itemFromRecord(record); this._callListeners("onItemAdded", item); let mm = Cc["@mozilla.org/globalmessagemanager;1"].getService(Ci.nsIMessageListenerManager); mm.broadcastAsyncMessage("Reader:Added", item); @@ -249,7 +254,7 @@ ReadingListImpl.prototype = { */ updateItem: Task.async(function* (item) { this._ensureItemBelongsToList(item); - yield this._store.updateItem(item._properties); + yield this._store.updateItem(item._record); this._invalidateIterators(); this._callListeners("onItemUpdated", item); }), @@ -268,26 +273,36 @@ ReadingListImpl.prototype = { this._ensureItemBelongsToList(item); yield this._store.deleteItemByURL(item.url); item.list = null; - this._itemsByURL.delete(item.url); + this._itemsByNormalizedURL.delete(item.url); this._invalidateIterators(); let mm = Cc["@mozilla.org/globalmessagemanager;1"].getService(Ci.nsIMessageListenerManager); mm.broadcastAsyncMessage("Reader:Removed", item); this._callListeners("onItemDeleted", item); }), + /** + * Finds the first item that matches the given options. + * + * @param optsList See Options Objects. + * @return The first matching item, or null if there are no matching items. + */ + item: Task.async(function* (...optsList) { + return (yield this.iterator(...optsList).items(1))[0] || null; + }), + /** * Find any item that matches a given URL - either the item's URL, or its * resolved URL. * * @param {String/nsIURI} uri - URI to match against. This will be normalized. + * @return The first matching item, or null if there are no matching items. */ - getItemForURL: Task.async(function* (uri) { + itemForURL: Task.async(function* (uri) { let url = normalizeURI(uri).spec; - let [item] = yield this.iterator({url: url}, {resolvedURL: url}).items(1); - return item; + return (yield this.item({ url: url }, { resolvedURL: url })); }), - /** + /** * Add to the ReadingList the page that is loaded in a given browser. * * @param {} browser - Browser element for the document, @@ -297,7 +312,7 @@ ReadingListImpl.prototype = { */ addItemFromBrowser: Task.async(function* (browser, url) { let metadata = yield getMetadataFromBrowser(browser); - let itemData = { + let record = { url: url, title: metadata.title, resolvedURL: metadata.url, @@ -305,11 +320,10 @@ ReadingListImpl.prototype = { }; if (metadata.previews.length > 0) { - itemData.preview = metadata.previews[0]; + record.preview = metadata.previews[0]; } - let item = yield ReadingList.addItem(itemData); - return item; + return (yield this.addItem(record)); }), /** @@ -340,21 +354,21 @@ ReadingListImpl.prototype = { */ destroy: Task.async(function* () { yield this._store.destroy(); - for (let itemWeakRef of this._itemsByURL.values()) { + for (let itemWeakRef of this._itemsByNormalizedURL.values()) { let item = itemWeakRef.get(); if (item) { item.list = null; } } - this._itemsByURL.clear(); + this._itemsByNormalizedURL.clear(); }), // The list's backing store. _store: null, - // A Map mapping URL strings to nsIWeakReferences that refer to + // A Map mapping *normalized* URL strings to nsIWeakReferences that refer to // ReadingListItems. - _itemsByURL: null, + _itemsByNormalizedURL: null, // A Set containing nsIWeakReferences that refer to valid iterators produced // by the list. @@ -364,22 +378,22 @@ ReadingListImpl.prototype = { _listeners: null, /** - * Returns the ReadingListItem represented by the given simple object. If + * Returns the ReadingListItem represented by the given record object. If * the item doesn't exist yet, it's created first. * - * @param obj A simple object with item properties. + * @param record A simple object with *normalized* item record properties. * @return The ReadingListItem. */ - _itemFromObject(obj) { - let itemWeakRef = this._itemsByURL.get(obj.url); + _itemFromRecord(record) { + let itemWeakRef = this._itemsByNormalizedURL.get(record.url); let item = itemWeakRef ? itemWeakRef.get() : null; if (item) { - item.setProperties(obj, false); + item._record = record; } else { - item = new ReadingListItem(obj); + item = new ReadingListItem(record); item.list = this; - this._itemsByURL.set(obj.url, Cu.getWeakReference(item)); + this._itemsByNormalizedURL.set(record.url, Cu.getWeakReference(item)); } return item; }, @@ -425,18 +439,6 @@ ReadingListImpl.prototype = { }, }; -/* - * normalize the properties of a "regular" object that reflects a ReadingListItem - */ -function normalizeReadingListProperties(obj) { - if (obj.url) { - obj.url = normalizeURI(obj.url).spec; - } - if (obj.resolvedURL) { - obj.resolvedURL = normalizeURI(obj.resolvedURL).spec; - } -} - let _unserializable = () => {}; // See comments in the ReadingListItem ctor. @@ -446,27 +448,30 @@ let _unserializable = () => {}; // See comments in the ReadingListItem ctor. * Each item belongs to a list, and it's an error to use an item with a * ReadingList that the item doesn't belong to. * - * @param props The properties of the item, as few or many as you want. + * @param record A simple object with the properties of the item, as few or many + * as you want. This will be normalized. */ -function ReadingListItem(props={}) { - this._properties = {}; +function ReadingListItem(record={}) { + this._record = record; // |this._unserializable| works around a problem when sending one of these // items via a message manager. If |this.list| is set, the item can't be // transferred directly, so .toJSON is implicitly called and the object // returned via that is sent. However, once the item is deleted and |this.list| // is null, the item *can* be directly serialized - so the message handler - // sees the "raw" object - ie, it sees "_properties" etc. + // sees the "raw" object - ie, it sees "_record" etc. // We work around this problem by *always* having an unserializable property // on the object - this way the implicit .toJSON call is always made, even // when |this.list| is null. this._unserializable = _unserializable; - - this.setProperties(props, false); } ReadingListItem.prototype = { + // Be careful when caching properties. If you cache a property that depends + // on a mutable _record property, then you need to recache your property after + // _record is set. + /** * Item's unique ID. * @type string @@ -480,27 +485,11 @@ ReadingListItem.prototype = { /** * The item's server-side GUID. This is set by the remote server and therefore is not - * guarenteed to be set for local items. + * guaranteed to be set for local items. * @type string */ get guid() { - return this._properties.guid || undefined; - }, - set guid(val) { - this._properties.guid = val; - }, - - /** - * The date the item was last modified. - * @type Date - */ - get lastModified() { - return this._properties.lastModified ? - new Date(this._properties.lastModified) : - undefined; - }, - set lastModified(val) { - this._properties.lastModified = val.valueOf(); + return this._record.guid || undefined; }, /** @@ -508,10 +497,7 @@ ReadingListItem.prototype = { * @type string */ get url() { - return this._properties.url; - }, - set url(val) { - this._properties.url = normalizeURI(val).spec; + return this._record.url; }, /** @@ -519,24 +505,12 @@ ReadingListItem.prototype = { * @type nsIURI */ get uri() { - return this._properties.url ? - Services.io.newURI(this._properties.url, "", null) : - undefined; - }, - set uri(val) { - this.url = normalizeURI(val).spec; - }, - - /** - * Returns the domain (a string) of the item's URL. If the URL doesn't have a - * domain, then the URL itself (also a string) is returned. - */ - get domain() { - try { - return this.uri.host; + if (!this._uri) { + this._uri = this._record.url ? + Services.io.newURI(this._record.url, "", null) : + undefined; } - catch (err) {} - return this.url; + return this._uri; }, /** @@ -544,23 +518,24 @@ ReadingListItem.prototype = { * @type string */ get resolvedURL() { - return this._properties.resolvedURL; + return this._record.resolvedURL; }, set resolvedURL(val) { - this._properties.resolvedURL = normalizeURI(val).spec; + this._updateRecord({ resolvedURL: val }); }, /** - * The item's resolved URL as an nsIURI. + * The item's resolved URL as an nsIURI. The setter takes an nsIURI or a + * string spec. * @type nsIURI */ get resolvedURI() { - return this._properties.resolvedURL ? - Services.io.newURI(this._properties.resolvedURL, "", null) : + return this._record.resolvedURL ? + Services.io.newURI(this._record.resolvedURL, "", null) : undefined; }, set resolvedURI(val) { - this.resolvedURL = val.spec; + this._updateRecord({ resolvedURL: val }); }, /** @@ -568,10 +543,10 @@ ReadingListItem.prototype = { * @type string */ get title() { - return this._properties.title; + return this._record.title; }, set title(val) { - this._properties.title = val; + this._updateRecord({ title: val }); }, /** @@ -579,10 +554,10 @@ ReadingListItem.prototype = { * @type string */ get resolvedTitle() { - return this._properties.resolvedTitle; + return this._record.resolvedTitle; }, set resolvedTitle(val) { - this._properties.resolvedTitle = val; + this._updateRecord({ resolvedTitle: val }); }, /** @@ -590,10 +565,10 @@ ReadingListItem.prototype = { * @type string */ get excerpt() { - return this._properties.excerpt; + return this._record.excerpt; }, set excerpt(val) { - this._properties.excerpt = val; + this._updateRecord({ excerpt: val }); }, /** @@ -601,10 +576,10 @@ ReadingListItem.prototype = { * @type integer */ get status() { - return this._properties.status; + return this._record.status; }, set status(val) { - this._properties.status = val; + this._updateRecord({ status: val }); }, /** @@ -612,10 +587,10 @@ ReadingListItem.prototype = { * @type boolean */ get favorite() { - return !!this._properties.favorite; + return !!this._record.favorite; }, set favorite(val) { - this._properties.favorite = !!val; + this._updateRecord({ favorite: !!val }); }, /** @@ -623,10 +598,10 @@ ReadingListItem.prototype = { * @type boolean */ get isArticle() { - return !!this._properties.isArticle; + return !!this._record.isArticle; }, set isArticle(val) { - this._properties.isArticle = !!val; + this._updateRecord({ isArticle: !!val }); }, /** @@ -634,10 +609,10 @@ ReadingListItem.prototype = { * @type integer */ get wordCount() { - return this._properties.wordCount; + return this._record.wordCount; }, set wordCount(val) { - this._properties.wordCount = val; + this._updateRecord({ wordCount: val }); }, /** @@ -645,10 +620,10 @@ ReadingListItem.prototype = { * @type boolean */ get unread() { - return !!this._properties.unread; + return !!this._record.unread; }, set unread(val) { - this._properties.unread = !!val; + this._updateRecord({ unread: !!val }); }, /** @@ -656,12 +631,12 @@ ReadingListItem.prototype = { * @type Date */ get addedOn() { - return this._properties.addedOn ? - new Date(this._properties.addedOn) : + return this._record.addedOn ? + new Date(this._record.addedOn) : undefined; }, set addedOn(val) { - this._properties.addedOn = val.valueOf(); + this._updateRecord({ addedOn: val.valueOf() }); }, /** @@ -669,12 +644,12 @@ ReadingListItem.prototype = { * @type Date */ get storedOn() { - return this._properties.storedOn ? - new Date(this._properties.storedOn) : + return this._record.storedOn ? + new Date(this._record.storedOn) : undefined; }, set storedOn(val) { - this._properties.storedOn = val.valueOf(); + this._updateRecord({ storedOn: val.valueOf() }); }, /** @@ -682,10 +657,10 @@ ReadingListItem.prototype = { * @type string */ get markedReadBy() { - return this._properties.markedReadBy; + return this._record.markedReadBy; }, set markedReadBy(val) { - this._properties.markedReadBy = val; + this._updateRecord({ markedReadBy: val }); }, /** @@ -693,12 +668,12 @@ ReadingListItem.prototype = { * @type Date */ get markedReadOn() { - return this._properties.markedReadOn ? - new Date(this._properties.markedReadOn) : + return this._record.markedReadOn ? + new Date(this._record.markedReadOn) : undefined; }, set markedReadOn(val) { - this._properties.markedReadOn = val.valueOf(); + this._updateRecord({ markedReadOn: val.valueOf() }); }, /** @@ -706,10 +681,10 @@ ReadingListItem.prototype = { * @param integer */ get readPosition() { - return this._properties.readPosition; + return this._record.readPosition; }, set readPosition(val) { - this._properties.readPosition = val; + this._updateRecord({ readPosition: val }); }, /** @@ -717,28 +692,9 @@ ReadingListItem.prototype = { * @type string */ get preview() { - return this._properties.preview; + return this._record.preview; }, - /** - * Sets the given properties of the item, optionally calling list.updateItem(). - * - * @param props A simple object containing the properties to set. - * @param update If true, updateItem() is called for this item. - * @return Promise If update is true, resolved when the update - * completes; otherwise resolved immediately. - */ - setProperties: Task.async(function* (props, update=true) { - for (let name in props) { - this._properties[name] = props[name]; - } - // make sure everything is normalized. - normalizeReadingListProperties(this._properties); - if (update) { - yield this.list.updateItem(this); - } - }), - /** * Deletes the item from its list. * @@ -751,7 +707,38 @@ ReadingListItem.prototype = { }), toJSON() { - return this._properties; + return this._record; + }, + + /** + * Do not use this at all unless you know what you're doing. Use the public + * getters and setters, above, instead. + * + * A simple object that contains the item's normalized data in the same format + * that the local store and server use. Records passed in by the consumer are + * not normalized, but everywhere else, records are always normalized unless + * otherwise stated. The setter normalizes the passed-in value, so it will + * throw an error if the value is not a valid record. + */ + get _record() { + return this.__record; + }, + set _record(val) { + this.__record = normalizeRecord(val); + }, + + /** + * Updates the item's record. This calls the _record setter, so it will throw + * an error if the partial record is not valid. + * + * @param partialRecord An object containing any of the record properties. + */ + _updateRecord(partialRecord) { + let record = this._record; + for (let prop in partialRecord) { + record[prop] = partialRecord[prop]; + } + this._record = record; }, _ensureBelongsToList() { @@ -854,6 +841,36 @@ ReadingListItemIterator.prototype = { }, }; + +/** + * Normalizes the properties of a record object, which represents a + * ReadingListItem. Throws an error if the record contains properties that + * aren't in ITEM_RECORD_PROPERTIES. + * + * @param record A non-normalized record object. + * @return The new normalized record. + */ +function normalizeRecord(nonNormalizedRecord) { + let record = {}; + for (let prop in nonNormalizedRecord) { + if (!ITEM_RECORD_PROPERTIES.includes(prop)) { + throw new Error("Unrecognized item property: " + prop); + } + switch (prop) { + case "url": + case "resolvedURL": + if (nonNormalizedRecord[prop]) { + record[prop] = normalizeURI(nonNormalizedRecord[prop]).spec; + } + break; + default: + record[prop] = nonNormalizedRecord[prop]; + break; + } + } + return record; +} + /** * Normalize a URI, stripping away extraneous parts we don't want to store * or compare against. @@ -872,16 +889,6 @@ function normalizeURI(uri) { return uri; }; -function stripNonItemProperties(item) { - let obj = {}; - for (let name of ITEM_BASIC_PROPERTY_NAMES) { - if (name in item) { - obj[name] = item[name]; - } - } - return obj; -} - function hash(str) { let hasher = Cc["@mozilla.org/security/hash;1"]. createInstance(Ci.nsICryptoHash); diff --git a/browser/components/readinglist/SQLiteStore.jsm b/browser/components/readinglist/SQLiteStore.jsm index 645bd21412cd..f7bc2357c42e 100644 --- a/browser/components/readinglist/SQLiteStore.jsm +++ b/browser/components/readinglist/SQLiteStore.jsm @@ -62,7 +62,7 @@ this.SQLiteStore.prototype = { */ forEachItem: Task.async(function* (callback, ...optsList) { let [sql, args] = sqlFromOptions(optsList); - let colNames = ReadingList.ItemBasicPropertyNames; + let colNames = ReadingList.ItemRecordProperties; let conn = yield this._connectionPromise; yield conn.executeCached(` SELECT ${colNames} FROM items ${sql}; @@ -71,7 +71,7 @@ this.SQLiteStore.prototype = { /** * Adds an item to the store that isn't already present. See - * ReadingList.prototype.addItems. + * ReadingList.prototype.addItem. * * @param items A simple object representing an item. * @return Promise Resolved when the store is updated. Rejected with an @@ -219,14 +219,14 @@ this.SQLiteStore.prototype = { /** * Returns a simple object whose properties are the - * ReadingList.ItemBasicPropertyNames properties lifted from the given row. + * ReadingList.ItemRecordProperties lifted from the given row. * * @param row A mozIStorageRow. * @return The item. */ function itemFromRow(row) { let item = {}; - for (let name of ReadingList.ItemBasicPropertyNames) { + for (let name of ReadingList.ItemRecordProperties) { item[name] = row.getResultByName(name); } return item; diff --git a/browser/components/readinglist/sidebar.js b/browser/components/readinglist/sidebar.js index b2d123e48ee0..085883cf1c20 100644 --- a/browser/components/readinglist/sidebar.js +++ b/browser/components/readinglist/sidebar.js @@ -61,9 +61,13 @@ let RLSidebar = { this.list.addEventListener("mousemove", event => this.onListMouseMove(event)); this.list.addEventListener("keydown", event => this.onListKeyDown(event), true); + window.addEventListener("message", event => this.onMessage(event)); + this.listPromise = this.ensureListItems(); ReadingList.addListener(this); + Services.prefs.setBoolPref("browser.readinglist.sidebarEverOpened", true); + let initEvent = new CustomEvent("Initialized", {bubbles: true}); document.documentElement.dispatchEvent(initEvent); }, @@ -84,12 +88,17 @@ let RLSidebar = { * * @param {ReadinglistItem} item - Item that was added. */ - onItemAdded(item) { + onItemAdded(item, append = false) { log.trace(`onItemAdded: ${item}`); let itemNode = document.importNode(this.itemTemplate.content, true).firstElementChild; this.updateItem(item, itemNode); - this.list.appendChild(itemNode); + // XXX Inserting at the top by default is a temp hack that will stop + // working once we start including items received from sync. + if (append) + this.list.appendChild(itemNode); + else + this.list.insertBefore(itemNode, this.list.firstChild); this.itemNodesById.set(item.id, itemNode); this.itemsById.set(item.id, item); @@ -138,7 +147,14 @@ let RLSidebar = { itemNode.setAttribute("title", `${item.title}\n${item.url}`); itemNode.querySelector(".item-title").textContent = item.title; - itemNode.querySelector(".item-domain").textContent = item.domain; + + let domain = item.uri.spec; + try { + domain = item.uri.host; + } + catch (err) {} + itemNode.querySelector(".item-domain").textContent = domain; + let thumb = itemNode.querySelector(".item-thumb-container"); if (item.preview) { thumb.style.backgroundImage = "url(" + item.preview + ")"; @@ -154,11 +170,11 @@ let RLSidebar = { yield ReadingList.forEachItem(item => { // TODO: Should be batch inserting via DocumentFragment try { - this.onItemAdded(item); + this.onItemAdded(item, true); } catch (e) { log.warn("Error adding item", e); } - }); + }, {sort: "addedOn", descending: true}); this.emptyListInfo.hidden = (this.numItems > 0); }), @@ -186,14 +202,8 @@ let RLSidebar = { log.debug(`Setting activeItem: ${node ? node.id : null}`); - if (node) { - if (!node.classList.contains("selected")) { - this.selectedItem = node; - } - - if (node.classList.contains("active")) { - return; - } + if (node && node.classList.contains("active")) { + return; } let prevItem = document.querySelector("#list > .item.active"); @@ -416,6 +426,26 @@ let RLSidebar = { } } }, + + /** + * Handle a message, typically sent from browser-readinglist.js + * @param {Event} event - Triggering event. + */ + onMessage(event) { + let msg = event.data; + + if (msg.topic != "UpdateActiveItem") { + return; + } + + if (!msg.url) { + this.activeItem = null; + } else { + ReadingList.itemForURL(msg.url).then(item => { + this.activeItem = this.itemNodesById.get(item.id); + }); + } + } }; diff --git a/browser/components/readinglist/test/ReadingListTestUtils.jsm b/browser/components/readinglist/test/ReadingListTestUtils.jsm index 30c91337dd1b..0494195b9dd0 100644 --- a/browser/components/readinglist/test/ReadingListTestUtils.jsm +++ b/browser/components/readinglist/test/ReadingListTestUtils.jsm @@ -84,7 +84,13 @@ SidebarUtils.prototype = { "Node should have correct title attribute"); this.Assert.equal(node.querySelector(".item-title").textContent, item.title, "Node's title element's text should match item title"); - this.Assert.equal(node.querySelector(".item-domain").textContent, item.domain, + + let domain = item.uri.spec; + try { + domain = item.uri.host; + } + catch (err) {} + this.Assert.equal(node.querySelector(".item-domain").textContent, domain, "Node's domain element's text should match item title"); }, diff --git a/browser/components/readinglist/test/xpcshell/test_ReadingList.js b/browser/components/readinglist/test/xpcshell/test_ReadingList.js index 6cc3be33d137..d770f66fc965 100644 --- a/browser/components/readinglist/test/xpcshell/test_ReadingList.js +++ b/browser/components/readinglist/test/xpcshell/test_ReadingList.js @@ -32,14 +32,12 @@ add_task(function* prepare() { gItems = []; for (let i = 0; i < 3; i++) { gItems.push({ - list: gList, guid: `guid${i}`, url: `http://example.com/${i}`, resolvedURL: `http://example.com/resolved/${i}`, title: `title ${i}`, excerpt: `excerpt ${i}`, unread: 0, - addedOn: Date.now(), lastModified: Date.now(), favorite: 0, isArticle: 1, @@ -63,14 +61,11 @@ add_task(function* item_properties() { Assert.ok(item.uri); Assert.ok(item.uri instanceof Ci.nsIURI); - Assert.equal(item.uri.spec, item.url); + Assert.equal(item.uri.spec, item._record.url); Assert.ok(item.resolvedURI); Assert.ok(item.resolvedURI instanceof Ci.nsIURI); - Assert.equal(item.resolvedURI.spec, item.resolvedURL); - - Assert.ok(item.lastModified); - Assert.ok(item.lastModified instanceof Cu.getGlobalForObject(ReadingList).Date); + Assert.equal(item.resolvedURI.spec, item._record.resolvedURL); Assert.ok(item.addedOn); Assert.ok(item.addedOn instanceof Cu.getGlobalForObject(ReadingList).Date); @@ -82,8 +77,7 @@ add_task(function* item_properties() { Assert.ok(typeof(item.isArticle) == "boolean"); Assert.ok(typeof(item.unread) == "boolean"); - Assert.equal(item.domain, "example.com"); - Assert.equal(item.id, hash(item.url)); + Assert.equal(item.id, hash(item._record.url)); }); add_task(function* constraints() { @@ -121,18 +115,6 @@ add_task(function* constraints() { } checkError(err); - // update an item with an existing url - let rlitem = yield gList.getItemForURL(gItems[0].url); - rlitem.guid = gItems[1].guid; - err = null; - try { - yield gList.updateItem(rlitem); - } - catch (e) { - err = e; - } - checkError(err); - // add a new item with an existing resolvedURL item = kindOfClone(gItems[0]); item.resolvedURL = gItems[0].resolvedURL; @@ -145,18 +127,32 @@ add_task(function* constraints() { } checkError(err); - // update an item with an existing resolvedURL - rlitem = yield gList.getItemForURL(gItems[0].url); - rlitem.url = gItems[1].url; + // add a new item with no url + item = kindOfClone(gItems[0]); + delete item.url; err = null; try { - yield gList.updateItem(rlitem); + yield gList.addItem(item); } catch (e) { err = e; } checkError(err); + // add an item with a bogus property + item = kindOfClone(gItems[0]); + item.bogus = "gnarly"; + err = null; + try { + yield gList.addItem(item); + } + catch (e) { + err = e; + } + Assert.ok(err); + Assert.ok(err.message); + Assert.ok(err.message.indexOf("Unrecognized item property:") >= 0); + // add a new item with no guid, which is allowed item = kindOfClone(gItems[0]); delete item.guid; @@ -183,24 +179,13 @@ add_task(function* constraints() { } Assert.ok(!err, err ? err.message : undefined); - // Delete both items since other tests assume the store contains only gItems. + // Delete the two previous items since other tests assume the store contains + // only gItems. yield gList.deleteItem(rlitem1); yield gList.deleteItem(rlitem2); let items = []; - yield gList.forEachItem(i => items.push(i), { url: [rlitem1.url, rlitem2.url] }); + yield gList.forEachItem(i => items.push(i), { url: [rlitem1.uri.spec, rlitem2.uri.spec] }); Assert.equal(items.length, 0); - - // add a new item with no url - item = kindOfClone(gItems[0]); - delete item.url; - err = null; - try { - yield gList.addItem(item); - } - catch (e) { - err = e; - } - checkError(err); }); add_task(function* count() { @@ -506,6 +491,22 @@ add_task(function* iterator_forEach_promise() { checkItems(items, gItems); }); +add_task(function* item() { + let item = yield gList.item({ guid: gItems[0].guid }); + checkItems([item], [gItems[0]]); + + item = yield gList.item({ guid: gItems[1].guid }); + checkItems([item], [gItems[1]]); +}); + +add_task(function* itemForURL() { + let item = yield gList.itemForURL(gItems[0].url); + checkItems([item], [gItems[0]]); + + item = yield gList.itemForURL(gItems[1].url); + checkItems([item], [gItems[1]]); +}); + add_task(function* updateItem() { // get an item let items = []; @@ -531,7 +532,7 @@ add_task(function* updateItem() { Assert.equal(item.title, newTitle); }); -add_task(function* item_setProperties() { +add_task(function* item_setRecord() { // get an item let iter = gList.iterator({ sort: "guid", @@ -539,12 +540,12 @@ add_task(function* item_setProperties() { let item = (yield iter.items(1))[0]; Assert.ok(item); - // item.setProperties(update=false). After fetching the item again, its title - // should be the old title. + // Set item._record without an updateItem. After fetching the item again, its + // title should be the old title. let oldTitle = item.title; - let newTitle = "item_setProperties title 1"; + let newTitle = "item_setRecord title 1"; Assert.notEqual(oldTitle, newTitle); - item.setProperties({ title: newTitle }, false); + item._record.title = newTitle; Assert.equal(item.title, newTitle); iter = gList.iterator({ sort: "guid", @@ -553,10 +554,11 @@ add_task(function* item_setProperties() { Assert.ok(item === sameItem); Assert.equal(sameItem.title, oldTitle); - // item.setProperties(update=true). After fetching the item again, its title - // should be the new title. - newTitle = "item_setProperties title 2"; - item.setProperties({ title: newTitle }, true); + // Set item._record followed by an updateItem. After fetching the item again, + // its title should be the new title. + newTitle = "item_setRecord title 2"; + item._record.title = newTitle; + yield gList.updateItem(item); Assert.equal(item.title, newTitle); iter = gList.iterator({ sort: "guid", @@ -565,11 +567,11 @@ add_task(function* item_setProperties() { Assert.ok(item === sameItem); Assert.equal(sameItem.title, newTitle); - // Set item.title directly. After fetching the item again, its title should - // be the new title. - newTitle = "item_setProperties title 3"; + // Set item.title directly and call updateItem. After fetching the item + // again, its title should be the new title. + newTitle = "item_setRecord title 3"; item.title = newTitle; - gList.updateItem(item); + yield gList.updateItem(item); Assert.equal(item.title, newTitle); iter = gList.iterator({ sort: "guid", @@ -577,6 +579,18 @@ add_task(function* item_setProperties() { sameItem = (yield iter.items(1))[0]; Assert.ok(item === sameItem); Assert.equal(sameItem.title, newTitle); + + // Setting _record to an object with a bogus property should throw. + let err = null; + try { + item._record = { bogus: "gnarly" }; + } + catch (e) { + err = e; + } + Assert.ok(err); + Assert.ok(err.message); + Assert.ok(err.message.indexOf("Unrecognized item property:") >= 0); }); add_task(function* listeners() { @@ -602,7 +616,7 @@ add_task(function* listeners() { }; gList.addListener(listener); items[0].title = "listeners new title"; - gList.updateItem(items[0]); + yield gList.updateItem(items[0]); let listenerItem = yield listenerPromise; Assert.ok(listenerItem); Assert.ok(listenerItem === items[0]); @@ -666,11 +680,10 @@ function checkItems(actualItems, expectedItems) { for (let i = 0; i < expectedItems.length; i++) { for (let prop in expectedItems[i]) { if (prop != "list") { - Assert.ok(prop in actualItems[i]._properties, prop); - Assert.equal(actualItems[i]._properties[prop], expectedItems[i][prop]); + Assert.ok(prop in actualItems[i]._record, prop); + Assert.equal(actualItems[i]._record[prop], expectedItems[i][prop]); } } - Assert.equal(actualItems[i].list, expectedItems[i].list); } } diff --git a/browser/locales/en-US/chrome/browser/newTab.dtd b/browser/locales/en-US/chrome/browser/newTab.dtd index 0ec53fad26d7..3b6aac6f34ac 100644 --- a/browser/locales/en-US/chrome/browser/newTab.dtd +++ b/browser/locales/en-US/chrome/browser/newTab.dtd @@ -5,9 +5,9 @@ - - - + + + diff --git a/browser/locales/en-US/chrome/browser/newTab.properties b/browser/locales/en-US/chrome/browser/newTab.properties index 3268f03148ca..162370f42c50 100644 --- a/browser/locales/en-US/chrome/browser/newTab.properties +++ b/browser/locales/en-US/chrome/browser/newTab.properties @@ -9,6 +9,11 @@ newtab.block=Remove this site # and enhanced tiles on the same line as the tile's title, so prefer short # strings to avoid overlap. This string should be uppercase. newtab.sponsored.button=SPONSORED +# LOCALIZATION NOTE(newtab.suggested.button): %1$S will be replaced inline by +# one of the user's top 100 sites that triggered this suggested tile. +# This text appears for suggested tiles under the tile's title, so prefer short +# strings to avoid truncating important text. +newtab.suggested.button=Suggested for %1$S visitors # LOCALIZATION NOTE(newtab.sponsored.explain): %1$S will be replaced inline by # the (X) block icon. %2$S will be replaced by an active link using string # newtab.learn.link as text. diff --git a/browser/modules/DirectoryLinksProvider.jsm b/browser/modules/DirectoryLinksProvider.jsm index be087c510d39..dc1eecd04f66 100644 --- a/browser/modules/DirectoryLinksProvider.jsm +++ b/browser/modules/DirectoryLinksProvider.jsm @@ -563,10 +563,18 @@ let DirectoryLinksProvider = { // from url to relatedLink. Thus, each link has an equal chance of being chosen at // random from flattenedLinks if it appears only once. let possibleLinks = new Map(); + let targetedSites = new Map(); this._topSitesWithRelatedLinks.forEach(topSiteWithRelatedLink => { let relatedLinksMap = this._relatedLinks.get(topSiteWithRelatedLink); relatedLinksMap.forEach((relatedLink, url) => { possibleLinks.set(url, relatedLink); + + // Keep a map of URL to targeted sites. We later use this to show the user + // what site they visited to trigger this suggestion. + if (!targetedSites.get(url)) { + targetedSites.set(url, []); + } + targetedSites.get(url).push(topSiteWithRelatedLink); }) }); let flattenedLinks = [...possibleLinks.values()]; @@ -578,9 +586,16 @@ let DirectoryLinksProvider = { // Show the new directory tile. this._callObservers("onLinkChanged", { url: chosenRelatedLink.url, + title: chosenRelatedLink.title, frecency: RELATED_FRECENCY, lastVisitDate: chosenRelatedLink.lastVisitDate, type: "related", + + // Choose the first site a user has visited as the target. In the future, + // this should be the site with the highest frecency. However, we currently + // store frecency by URL not by site. + targetedSite: targetedSites.get(chosenRelatedLink.url).length ? + targetedSites.get(chosenRelatedLink.url)[0] : null }); return chosenRelatedLink; }, diff --git a/browser/modules/ReaderParent.jsm b/browser/modules/ReaderParent.jsm index 77fa6aa876f0..7ca2f86495e4 100644 --- a/browser/modules/ReaderParent.jsm +++ b/browser/modules/ReaderParent.jsm @@ -60,7 +60,7 @@ let ReaderParent = { break; } case "Reader:ListStatusRequest": - ReadingList.containsURL(message.data.url).then(inList => { + ReadingList.hasItemForURL(message.data.url).then(inList => { let mm = message.target.messageManager // Make sure the target browser is still alive before trying to send data back. if (mm) { @@ -72,7 +72,7 @@ let ReaderParent = { case "Reader:RemoveFromList": // We need to get the "real" item to delete it. - ReadingList.getItemForURL(message.data.url).then(item => { + ReadingList.itemForURL(message.data.url).then(item => { ReadingList.deleteItem(item) }); break; diff --git a/browser/themes/shared/newtab/newTab.inc.css b/browser/themes/shared/newtab/newTab.inc.css index 0ff83516712c..b7965d92e2b9 100644 --- a/browser/themes/shared/newtab/newTab.inc.css +++ b/browser/themes/shared/newtab/newTab.inc.css @@ -122,7 +122,7 @@ transition: opacity 100ms ease-out; } -.newtab-site:hover .newtab-thumbnail.enhanced-content { +.newtab-cell:not([ignorehover]) .newtab-site:hover .newtab-thumbnail.enhanced-content { opacity: 0; } @@ -137,10 +137,15 @@ /* TITLES */ #newtab-intro-what, .newtab-sponsored, -.newtab-title { +.newtab-title, +.newtab-suggested { color: #5c5c5c; } +.newtab-suggested { + background-color: white; +} + .newtab-site:hover .newtab-title { color: #222; } diff --git a/browser/themes/shared/readinglist/sidebar.inc.css b/browser/themes/shared/readinglist/sidebar.inc.css index da49e55dfefc..32912a595a3e 100644 --- a/browser/themes/shared/readinglist/sidebar.inc.css +++ b/browser/themes/shared/readinglist/sidebar.inc.css @@ -51,7 +51,7 @@ body { box-shadow: 0px 1px 2px rgba(0,0,0,.35); margin: 5px; background-color: #fff; - background-size: contain; + background-size: cover; background-repeat: no-repeat; background-position: center; background-image: url("chrome://branding/content/silhouette-40.svg"); diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h index 62de0dc4b3a2..0fc5e9985dc4 100644 --- a/gfx/thebes/gfxPlatform.h +++ b/gfx/thebes/gfxPlatform.h @@ -166,7 +166,8 @@ enum class DeviceResetReason REMOVED, RESET, DRIVER_ERROR, - INVALID_CALL + INVALID_CALL, + OUT_OF_MEMORY }; class gfxPlatform { diff --git a/gfx/thebes/gfxWindowsPlatform.cpp b/gfx/thebes/gfxWindowsPlatform.cpp index cf10a4b575fa..8bbb66b8f57d 100644 --- a/gfx/thebes/gfxWindowsPlatform.cpp +++ b/gfx/thebes/gfxWindowsPlatform.cpp @@ -1168,6 +1168,10 @@ gfxWindowsPlatform::DidRenderingDeviceReset(DeviceResetReason* aResetReason) break; case DXGI_ERROR_INVALID_CALL: *aResetReason = DeviceResetReason::INVALID_CALL; + break; + case E_OUTOFMEMORY: + *aResetReason = DeviceResetReason::OUT_OF_MEMORY; + break; default: MOZ_ASSERT(false); } diff --git a/mobile/android/base/tests/testAboutPasswords.js b/mobile/android/base/tests/testAboutPasswords.js index a33ed11cd5b5..d06f0d5d84f8 100644 --- a/mobile/android/base/tests/testAboutPasswords.js +++ b/mobile/android/base/tests/testAboutPasswords.js @@ -63,39 +63,7 @@ add_test(function test_passwords_list() { let username = logins_list.querySelector(".username"); do_check_eq(username.textContent, LOGIN_FIELDS.username); - let login_item = browser.contentDocument.querySelector("#logins-list > .login-item"); - browser.addEventListener("PasswordsDetailsLoad", function() { - browser.removeEventListener("PasswordsDetailsLoad", this, false); - Services.tm.mainThread.dispatch(run_next_test, Ci.nsIThread.DISPATCH_NORMAL); - }, false); - - // Expand item details. - login_item.click(); -}); - -add_test(function test_passwords_details() { - let login_details = browser.contentDocument.getElementById("login-details"); - - let hostname = login_details.querySelector(".hostname"); - do_check_eq(hostname.textContent, LOGIN_FIELDS.hostname); - let username = login_details.querySelector(".username"); - do_check_eq(username.textContent, LOGIN_FIELDS.username); - - // Check that details page opens link to host. - BrowserApp.deck.addEventListener("TabOpen", (tabevent) => { - // Wait for tab to finish loading. - let browser_target = tabevent.target; - browser_target.addEventListener("load", () => { - browser_target.removeEventListener("load", this, true); - - do_check_eq(BrowserApp.selectedTab.browser.currentURI.spec, LOGIN_FIELDS.hostname); - Services.tm.mainThread.dispatch(run_next_test, Ci.nsIThread.DISPATCH_NORMAL); - }, true); - - BrowserApp.deck.removeEventListener("TabOpen", this, false); - }, false); - - browser.contentDocument.getElementById("details-header").click(); + run_next_test(); }); run_next_test(); diff --git a/mobile/android/chrome/content/aboutPasswords.js b/mobile/android/chrome/content/aboutPasswords.js index 0492bb9a4453..169ef31ce60d 100644 --- a/mobile/android/chrome/content/aboutPasswords.js +++ b/mobile/android/chrome/content/aboutPasswords.js @@ -17,6 +17,9 @@ XPCOMUtils.defineLazyGetter(window, "gChromeWin", function() .getInterface(Ci.nsIDOMWindow) .QueryInterface(Ci.nsIDOMChromeWindow)); +XPCOMUtils.defineLazyModuleGetter(this, "Prompt", + "resource://gre/modules/Prompt.jsm"); + let debug = Cu.import("resource://gre/modules/AndroidLog.jsm", {}).AndroidLog.d.bind(null, "AboutPasswords"); let gStringBundle = Services.strings.createBundle("chrome://browser/locale/aboutPasswords.properties"); @@ -125,9 +128,33 @@ let Passwords = { loginItem.setAttribute("loginID", login.guid); loginItem.className = "login-item list-item"; + loginItem.addEventListener("click", () => { - this._showDetails(loginItem); - history.pushState({ id: login.guid }, document.title); + let prompt = new Prompt({ + window: window, + }); + let menuItems = [ + { label: gStringBundle.GetStringFromName("passwordsMenu.copyPassword") }, + { label: gStringBundle.GetStringFromName("passwordsMenu.copyUsername") }, + { label: gStringBundle.GetStringFromName("passwordsMenu.details") } ]; + + prompt.setSingleChoiceItems(menuItems); + prompt.show((data) => { + // Switch on indices of buttons, as they were added when creating login item. + switch (data.button) { + case 0: + copyStringAndToast(login.password, gStringBundle.GetStringFromName("passwordsDetails.passwordCopied")); + break; + case 1: + copyStringAndToast(login.username, gStringBundle.GetStringFromName("passwordsDetails.usernameCopied")); + break; + case 2: + this._showDetails(loginItem); + history.pushState({ id: login.guid }, document.title); + break; + } + }); + }, true); // Create item icon. diff --git a/mobile/android/locales/en-US/chrome/aboutPasswords.properties b/mobile/android/locales/en-US/chrome/aboutPasswords.properties index 386ed4136ef1..02f206a09951 100644 --- a/mobile/android/locales/en-US/chrome/aboutPasswords.properties +++ b/mobile/android/locales/en-US/chrome/aboutPasswords.properties @@ -2,6 +2,10 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +passwordsMenu.copyPassword=Copy password +passwordsMenu.copyUsername=Copy username +passwordsMenu.details=Details + passwordsDetails.age=Age: %S days passwordsDetails.copyFailed=Copy failed diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 170b7efc9103..5c74d25cb747 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -214,7 +214,7 @@ "expires_in_version": "never", "kind": "enumerated", "n_values": 10, - "description": "GPU Device Reset Reason (ok, hung, removed, reset, internal error, invalid call)" + "description": "GPU Device Reset Reason (ok, hung, removed, reset, internal error, invalid call, out of memory)" }, "FORGET_SKIPPABLE_MAX": { "expires_in_version": "never", diff --git a/toolkit/modules/NewTabUtils.jsm b/toolkit/modules/NewTabUtils.jsm index d51ce2b5aa27..e8d9030c861c 100644 --- a/toolkit/modules/NewTabUtils.jsm +++ b/toolkit/modules/NewTabUtils.jsm @@ -935,7 +935,12 @@ let Links = { _getMergedProviderLinks: function Links__getMergedProviderLinks() { // Build a list containing a copy of each provider's sortedLinks list. let linkLists = []; - for (let links of this._providers.values()) { + for (let provider of this._providers.keys()) { + if (!AllPages.enhanced && provider != PlacesProvider) { + // Only show history tiles if we're not in 'enhanced' mode. + continue; + } + let links = this._providers.get(provider); if (links && links.sortedLinks) { linkLists.push(links.sortedLinks.slice()); } @@ -1248,11 +1253,19 @@ this.NewTabUtils = { }, getProviderLinks: function(aProvider) { - return Links._providers.get(aProvider).sortedLinks; + let cache = Links._providers.get(aProvider); + if (cache && cache.sortedLinks) { + return cache.sortedLinks; + } + return []; }, isTopSiteGivenProvider: function(aSite, aProvider) { - return Links._providers.get(aProvider).siteMap.has(aSite); + let cache = Links._providers.get(aProvider); + if (cache && cache.siteMap) { + return cache.siteMap.has(aSite); + } + return false; }, isTopPlacesSite: function(aSite) { diff --git a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js index 2d4592fa20eb..fa87b788ccb8 100644 --- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js +++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js @@ -7,11 +7,38 @@ const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components; Cu.import("resource://gre/modules/NewTabUtils.jsm"); Cu.import("resource://gre/modules/Promise.jsm"); Cu.import("resource://gre/modules/Task.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); + +const PREF_NEWTAB_ENHANCED = "browser.newtabpage.enhanced"; function run_test() { + Services.prefs.setBoolPref(PREF_NEWTAB_ENHANCED, true); run_next_test(); } +add_task(function validCacheMidPopulation() { + let expectedLinks = makeLinks(0, 3, 1); + + let provider = new TestProvider(done => done(expectedLinks)); + provider.maxNumLinks = expectedLinks.length; + + NewTabUtils.initWithoutProviders(); + NewTabUtils.links.addProvider(provider); + let promise = new Promise(resolve => NewTabUtils.links.populateCache(resolve)); + + // isTopSiteGivenProvider() and getProviderLinks() should still return results + // even when cache is empty or being populated. + do_check_false(NewTabUtils.isTopSiteGivenProvider("example1.com", provider)); + do_check_links(NewTabUtils.getProviderLinks(provider), []); + + yield promise; + + // Once the cache is populated, we get the expected results + do_check_true(NewTabUtils.isTopSiteGivenProvider("example1.com", provider)); + do_check_links(NewTabUtils.getProviderLinks(provider), expectedLinks); + NewTabUtils.links.removeProvider(provider); +}); + add_task(function notifyLinkDelete() { let expectedLinks = makeLinks(0, 3, 1);