diff --git a/browser/base/content/test/urlbar/browser_canonizeURL.js b/browser/base/content/test/urlbar/browser_canonizeURL.js index c301589d40b1..40514ea7e2b8 100644 --- a/browser/base/content/test/urlbar/browser_canonizeURL.js +++ b/browser/base/content/test/urlbar/browser_canonizeURL.js @@ -33,14 +33,8 @@ add_task(async function checkCtrlWorks() { for (let [inputValue, expectedURL, options] of testcases) { let promiseLoad = waitForDocLoadAndStopIt(expectedURL); gURLBar.focus(); - if (Object.keys(options).length > 0) { - gURLBar.selectionStart = gURLBar.selectionEnd = - gURLBar.inputField.value.length; - gURLBar.inputField.value = inputValue.slice(0, -1); - EventUtils.sendString(inputValue.slice(-1)); - } else { - gURLBar.textValue = inputValue; - } + gURLBar.inputField.value = inputValue.slice(0, -1); + EventUtils.sendString(inputValue.slice(-1)); EventUtils.synthesizeKey("KEY_Enter", options); await promiseLoad; } diff --git a/browser/base/content/test/urlbar/browser_urlbarDecode.js b/browser/base/content/test/urlbar/browser_urlbarDecode.js index 2ed13b9433d4..1a235b688749 100644 --- a/browser/base/content/test/urlbar/browser_urlbarDecode.js +++ b/browser/base/content/test/urlbar/browser_urlbarDecode.js @@ -5,6 +5,11 @@ // the urlbar also shows the URLs embedded in action URIs unescaped. See bug // 1233672. +XPCOMUtils.defineLazyModuleGetters(this, { + UrlbarMatch: "resource:///modules/UrlbarMatch.jsm", + UrlbarUtils: "resource:///modules/UrlbarUtils.jsm", +}); + add_task(async function injectJSON() { let inputStrs = [ 'http://example.com/ ", "url": "bar', @@ -27,7 +32,12 @@ add_task(async function injectJSON() { add_task(function losslessDecode() { let urlNoScheme = "example.com/\u30a2\u30a4\u30a6\u30a8\u30aa"; let url = "http://" + urlNoScheme; - gURLBar.textValue = url; + if (Services.prefs.getBoolPref("browser.urlbar.quantumbar", true)) { + const result = new UrlbarMatch(UrlbarUtils.MATCH_TYPE.TAB_SWITCH, {url}); + gURLBar.setValueFromResult(result); + } else { + gURLBar.textValue = url; + } // Since this is directly setting textValue, it is expected to be trimmed. Assert.equal(gURLBar.inputField.value, urlNoScheme, "The string displayed in the textbox should not be escaped"); diff --git a/browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js b/browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js index b92820dfea67..df66ad15d9d6 100644 --- a/browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js +++ b/browser/components/extensions/test/browser/browser_ext_webNavigation_urlbar_transitions.js @@ -109,8 +109,9 @@ add_task(async function test_webnavigation_urlbar_typed_transitions() { await extension.awaitMessage("ready"); gURLBar.focus(); - gURLBar.textValue = "http://example.com/?q=typed"; - + const inputValue = "http://example.com/?q=typed"; + gURLBar.inputField.value = inputValue.slice(0, -1); + EventUtils.sendString(inputValue.slice(-1)); EventUtils.synthesizeKey("VK_RETURN", {altKey: true}); await extension.awaitFinish("webNavigation.from_address_bar.typed"); diff --git a/browser/components/originattributes/test/browser/browser.ini b/browser/components/originattributes/test/browser/browser.ini index ace5a1a438ce..1fc7286fab7d 100644 --- a/browser/components/originattributes/test/browser/browser.ini +++ b/browser/components/originattributes/test/browser/browser.ini @@ -12,6 +12,8 @@ support-files = file_favicon_cache.png file_favicon_thirdParty.html file_firstPartyBasic.html + file_postMessage.html + file_postMessageSender.html file_sharedworker.html file_sharedworker.js file_thirdPartyChild.audio.ogg @@ -80,5 +82,6 @@ skip-if = (verify && debug && (os == 'win')) skip-if = verify [browser_cacheAPI.js] [browser_permissions.js] +[browser_postMessage.js] [browser_sanitize.js] [browser_windowOpenerRestriction.js] diff --git a/browser/components/originattributes/test/browser/browser_postMessage.js b/browser/components/originattributes/test/browser/browser_postMessage.js new file mode 100644 index 000000000000..899632948f33 --- /dev/null +++ b/browser/components/originattributes/test/browser/browser_postMessage.js @@ -0,0 +1,96 @@ +/** + * Bug 1492607 - Test for assuring that postMessage cannot go across OAs. + */ + +const FPD_ONE = "http://example.com"; +const FPD_TWO = "http://example.org"; + +const TEST_BASE = "/browser/browser/components/originattributes/test/browser/"; + +add_task(async function setup() { + // Make sure first party isolation is enabled. + await SpecialPowers.pushPrefEnv({"set": [ + ["privacy.firstparty.isolate", true], + ]}); +}); + +async function runTestWithOptions(aDifferentFPD, aStarTargetOrigin, aBlockAcrossFPD) { + let testPageURL = aDifferentFPD ? + FPD_ONE + TEST_BASE + "file_postMessage.html" : + FPD_TWO + TEST_BASE + "file_postMessage.html"; + + // Deciding the targetOrigin according to the test setting. + let targetOrigin; + if (aStarTargetOrigin) { + targetOrigin = "*"; + } else { + targetOrigin = aDifferentFPD ? FPD_ONE : FPD_TWO; + } + let senderURL = FPD_TWO + TEST_BASE + `file_postMessageSender.html?${targetOrigin}`; + + // Open a tab to listen messages. + let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, testPageURL); + + // Use window.open() in the tab to open the sender tab. The sender tab + // will send a message through postMessage to window.opener. + let senderTabPromise = BrowserTestUtils.waitForNewTab(gBrowser, senderURL, true); + ContentTask.spawn(tab.linkedBrowser, senderURL, + aSenderPath => { + content.open(aSenderPath, "_blank"); + } + ); + + // Wait and get the tab of the sender tab. + let senderTab = await senderTabPromise; + + // The postMessage should be blocked when the first parties are different with + // the following two cases. First, it is using a non-star target origin. + // Second, it is using the star target origin and the pref + // 'privacy.firstparty.isolate.block_post_message' is true. + let shouldBlock = aDifferentFPD && (!aStarTargetOrigin || aBlockAcrossFPD); + + await ContentTask.spawn(tab.linkedBrowser, shouldBlock, async (aValue) => { + await new Promise(resolve => { + content.addEventListener("message", function eventHandler(aEvent) { + if (aEvent.data === "Self") { + if (aValue) { + is(content.document.getElementById("display").innerHTML, "", + "It should not get a message from other OA."); + } else { + is(content.document.getElementById("display").innerHTML, "Message", + "It should get a message from the same OA."); + } + + content.removeEventListener("message", eventHandler); + resolve(); + } + }); + + // Trigger the content to send a postMessage to itself. + content.document.getElementById("button").click(); + }); + }); + + BrowserTestUtils.removeTab(tab); + BrowserTestUtils.removeTab(senderTab); +} + +add_task(async function runTests() { + for (let useDifferentFPD of [true, false]) { + for (let useStarTargetOrigin of [true, false]) { + for (let enableBlocking of [true, false]) { + if (enableBlocking) { + await SpecialPowers.pushPrefEnv({"set": [ + ["privacy.firstparty.isolate.block_post_message", true], + ]}); + } + + await runTestWithOptions(useDifferentFPD, useStarTargetOrigin, enableBlocking); + + if (enableBlocking) { + await SpecialPowers.popPrefEnv(); + } + } + } + } +}); diff --git a/browser/components/originattributes/test/browser/file_postMessage.html b/browser/components/originattributes/test/browser/file_postMessage.html new file mode 100644 index 000000000000..91255d03648a --- /dev/null +++ b/browser/components/originattributes/test/browser/file_postMessage.html @@ -0,0 +1,27 @@ + + + + + Test page for window.postMessage + + +
+ + + + diff --git a/browser/components/originattributes/test/browser/file_postMessageSender.html b/browser/components/originattributes/test/browser/file_postMessageSender.html new file mode 100644 index 000000000000..ebdffb990fb5 --- /dev/null +++ b/browser/components/originattributes/test/browser/file_postMessageSender.html @@ -0,0 +1,16 @@ + + + + + Test page for always sending a message to its opener through postMessage + + + + + diff --git a/browser/components/places/PlacesUIUtils.jsm b/browser/components/places/PlacesUIUtils.jsm index c4bd29221a0b..033cad50b6e9 100644 --- a/browser/components/places/PlacesUIUtils.jsm +++ b/browser/components/places/PlacesUIUtils.jsm @@ -518,11 +518,9 @@ var PlacesUIUtils = { * * @param aNode * a node, except the root node of a query. - * @param aView - * The view originating the request. * @return true if the aNode represents a removable entry, false otherwise. */ - canUserRemove(aNode, aView) { + canUserRemove(aNode) { let parentNode = aNode.parent; if (!parentNode) { // canUserRemove doesn't accept root nodes. @@ -558,7 +556,7 @@ var PlacesUIUtils = { return true; // Otherwise it has to be a child of an editable folder. - return !this.isFolderReadOnly(parentNode, aView); + return !this.isFolderReadOnly(parentNode); }, /** @@ -576,25 +574,15 @@ var PlacesUIUtils = { * * @param placesNode * any folder result node. - * @param view - * The view originating the request. * @throws if placesNode is not a folder result node or views is invalid. - * @note livemark "folders" are considered read-only (but see bug 1072833). * @return true if placesNode is a read-only folder, false otherwise. */ - isFolderReadOnly(placesNode, view) { + isFolderReadOnly(placesNode) { if (typeof placesNode != "object" || !PlacesUtils.nodeIsFolder(placesNode)) { throw new Error("invalid value for placesNode"); } - if (!view || typeof view != "object") { - throw new Error("invalid value for aView"); - } - let itemId = PlacesUtils.getConcreteItemId(placesNode); - if (itemId == PlacesUtils.placesRootId || - view.controller.hasCachedLivemarkInfo(placesNode)) - return true; - return false; + return PlacesUtils.getConcreteItemId(placesNode) == PlacesUtils.placesRootId; }, /** aItemsToOpen needs to be an array of objects of the form: @@ -646,25 +634,6 @@ var PlacesUIUtils = { }); }, - openLiveMarkNodesInTabs: - function PUIU_openLiveMarkNodesInTabs(aNode, aEvent, aView) { - let window = aView.ownerWindow; - - PlacesUtils.livemarks.getLivemark({id: aNode.itemId}) - .then(aLivemark => { - let urlsToOpen = []; - - let nodes = aLivemark.getNodesForContainer(aNode); - for (let node of nodes) { - urlsToOpen.push({uri: node.uri, isBookmark: false}); - } - - if (OpenInTabsUtils.confirmOpenInTabs(urlsToOpen.length, window)) { - this._openTabset(urlsToOpen, aEvent, window); - } - }, Cu.reportError); - }, - openContainerNodeInTabs: function PUIU_openContainerInTabs(aNode, aEvent, aView) { let window = aView.ownerWindow; diff --git a/browser/components/places/content/browserPlacesViews.js b/browser/components/places/content/browserPlacesViews.js index 2743d92c78da..2b5309f60c49 100644 --- a/browser/components/places/content/browserPlacesViews.js +++ b/browser/components/places/content/browserPlacesViews.js @@ -273,13 +273,6 @@ PlacesViewBase.prototype = { if (!resultNode.containerOpen) return; - if (this.controller.hasCachedLivemarkInfo(resultNode)) { - this._setEmptyPopupStatus(aPopup, false); - aPopup._built = true; - this._populateLivemarkPopup(aPopup); - return; - } - this._cleanPopup(aPopup); let cc = resultNode.childCount; @@ -343,7 +336,6 @@ PlacesViewBase.prototype = { element = document.createXULElement("menuseparator"); element.setAttribute("class", "small-separator"); } else { - let itemId = aPlacesNode.itemId; if (type == Ci.nsINavHistoryResultNode.RESULT_TYPE_URI) { element = document.createXULElement("menuitem"); element.className = "menuitem-iconic bookmark-item menuitem-with-favicon"; @@ -361,18 +353,6 @@ PlacesViewBase.prototype = { element.setAttribute("dayContainer", "true"); else if (PlacesUtils.nodeIsHost(aPlacesNode)) element.setAttribute("hostContainer", "true"); - } else if (itemId != -1) { - PlacesUtils.livemarks.getLivemark({ id: itemId }) - .then(aLivemark => { - element.setAttribute("livemark", "true"); - if (AppConstants.platform === "macosx") { - // OS X native menubar doesn't track list-style-images since - // it doesn't have a frame (bug 733415). Thus enforce updating. - element.setAttribute("image", ""); - element.removeAttribute("image"); - } - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - }, () => undefined); } let popup = document.createXULElement("menupopup"); @@ -419,80 +399,6 @@ PlacesViewBase.prototype = { return element; }, - _setLivemarkSiteURIMenuItem: - function PVB__setLivemarkSiteURIMenuItem(aPopup) { - let livemarkInfo = this.controller.getCachedLivemarkInfo(aPopup._placesNode); - let siteUrl = livemarkInfo && livemarkInfo.siteURI ? - livemarkInfo.siteURI.spec : null; - if (!siteUrl && aPopup._siteURIMenuitem) { - aPopup.removeChild(aPopup._siteURIMenuitem); - aPopup._siteURIMenuitem = null; - aPopup.removeChild(aPopup._siteURIMenuseparator); - aPopup._siteURIMenuseparator = null; - } else if (siteUrl && !aPopup._siteURIMenuitem) { - // Add "Open (Feed Name)" menuitem. - aPopup._siteURIMenuitem = document.createXULElement("menuitem"); - aPopup._siteURIMenuitem.className = "openlivemarksite-menuitem"; - if (typeof this.options.extraClasses.entry == "string") { - aPopup._siteURIMenuitem.classList.add(this.options.extraClasses.entry); - } - aPopup._siteURIMenuitem.setAttribute("targetURI", siteUrl); - aPopup._siteURIMenuitem.setAttribute("oncommand", - "openUILink(this.getAttribute('targetURI'), event, {" + - " triggeringPrincipal: Services.scriptSecurityManager.createNullPrincipal({})});"); - - // If a user middle-clicks this item we serve the oncommand event. - // We are using checkForMiddleClick because of Bug 246720. - // Note: stopPropagation is needed to avoid serving middle-click - // with BT_onClick that would open all items in tabs. - aPopup._siteURIMenuitem.setAttribute("onclick", - "checkForMiddleClick(this, event); event.stopPropagation();"); - let label = - PlacesUIUtils.getFormattedString("menuOpenLivemarkOrigin.label", - [aPopup.parentNode.getAttribute("label")]); - aPopup._siteURIMenuitem.setAttribute("label", label); - aPopup.insertBefore(aPopup._siteURIMenuitem, aPopup._startMarker); - - aPopup._siteURIMenuseparator = document.createXULElement("menuseparator"); - aPopup.insertBefore(aPopup._siteURIMenuseparator, aPopup._startMarker); - } - }, - - /** - * Add, update or remove the livemark status menuitem. - * @param aPopup - * The livemark container popup - * @param aStatus - * The livemark status - */ - _setLivemarkStatusMenuItem: - function PVB_setLivemarkStatusMenuItem(aPopup, aStatus) { - let statusMenuitem = aPopup._statusMenuitem; - if (!statusMenuitem) { - // Create the status menuitem and cache it in the popup object. - statusMenuitem = document.createXULElement("menuitem"); - statusMenuitem.className = "livemarkstatus-menuitem"; - if (typeof this.options.extraClasses.entry == "string") { - statusMenuitem.classList.add(this.options.extraClasses.entry); - } - statusMenuitem.setAttribute("disabled", true); - aPopup._statusMenuitem = statusMenuitem; - } - - if (aStatus == Ci.mozILivemark.STATUS_LOADING || - aStatus == Ci.mozILivemark.STATUS_FAILED) { - // Status has changed, update the cached status menuitem. - let stringId = aStatus == Ci.mozILivemark.STATUS_LOADING ? - "bookmarksLivemarkLoading" : "bookmarksLivemarkFailed"; - statusMenuitem.setAttribute("label", PlacesUIUtils.getString(stringId)); - if (aPopup._startMarker.nextElementSibling != statusMenuitem) - aPopup.insertBefore(statusMenuitem, aPopup._startMarker.nextElementSibling); - } else if (aPopup._statusMenuitem.parentNode == aPopup) { - // The livemark has finished loading. - aPopup.removeChild(aPopup._statusMenuitem); - } - }, - toggleCutNode: function PVB_toggleCutNode(aPlacesNode, aValue) { let elt = this._getDOMNodeForPlacesNode(aPlacesNode); @@ -532,32 +438,7 @@ PlacesViewBase.prototype = { elt.setAttribute("image", aPlacesNode.icon); }, - nodeAnnotationChanged: - function PVB_nodeAnnotationChanged(aPlacesNode, aAnno) { - let elt = this._getDOMNodeForPlacesNode(aPlacesNode); - - // All livemarks have a feedURI, so use it as our indicator of a livemark - // being modified. - if (aAnno == PlacesUtils.LMANNO_FEEDURI) { - let menu = elt.parentNode; - if (!menu.hasAttribute("livemark")) { - menu.setAttribute("livemark", "true"); - if (AppConstants.platform === "macosx") { - // OS X native menubar doesn't track list-style-images since - // it doesn't have a frame (bug 733415). Thus enforce updating. - menu.setAttribute("image", ""); - menu.removeAttribute("image"); - } - } - - PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) - .then(aLivemark => { - // Controller will use this to build the meta data for the node. - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - this.invalidateContainer(aPlacesNode); - }, () => undefined); - } - }, + nodeAnnotationChanged() {}, nodeTitleChanged: function PVB_nodeTitleChanged(aPlacesNode, aNewTitle) { @@ -602,26 +483,7 @@ PlacesViewBase.prototype = { } }, - nodeHistoryDetailsChanged: - function PVB_nodeHistoryDetailsChanged(aPlacesNode, aTime, aCount) { - if (aPlacesNode.parent && - this.controller.hasCachedLivemarkInfo(aPlacesNode.parent)) { - // Find the node in the parent. - let popup = this._getDOMNodeForPlacesNode(aPlacesNode.parent); - for (let child = popup._startMarker.nextElementSibling; - child != popup._endMarker; - child = child.nextElementSibling) { - if (child._placesNode && child._placesNode.uri == aPlacesNode.uri) { - if (aPlacesNode.accessCount) - child.setAttribute("visited", "true"); - else - child.removeAttribute("visited"); - break; - } - } - } - }, - + nodeHistoryDetailsChanged() { }, nodeTagsChanged() { }, nodeDateAddedChanged() { }, nodeLastModifiedChanged() { }, @@ -676,66 +538,9 @@ PlacesViewBase.prototype = { if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED || aNewState == Ci.nsINavHistoryContainerResultNode.STATE_CLOSED) { this.invalidateContainer(aPlacesNode); - - if (PlacesUtils.nodeIsFolder(aPlacesNode)) { - let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions; - if (queryOptions.excludeItems) { - return; - } - - PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) - .then(aLivemark => { - if (!this.controller) { - // We might have been destroyed in the interim... - return; - } - let shouldInvalidate = - !this.controller.hasCachedLivemarkInfo(aPlacesNode); - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED) { - aLivemark.registerForUpdates(aPlacesNode, this); - // Prioritize the current livemark. - aLivemark.reload(); - PlacesUtils.livemarks.reloadLivemarks(); - if (shouldInvalidate) - this.invalidateContainer(aPlacesNode); - } else { - aLivemark.unregisterForUpdates(aPlacesNode); - } - }, () => undefined); - } } }, - _populateLivemarkPopup: function PVB__populateLivemarkPopup(aPopup) { - this._setLivemarkSiteURIMenuItem(aPopup); - // Show the loading status only if there are no entries yet. - if (aPopup._startMarker.nextElementSibling == aPopup._endMarker) - this._setLivemarkStatusMenuItem(aPopup, Ci.mozILivemark.STATUS_LOADING); - - PlacesUtils.livemarks.getLivemark({ id: aPopup._placesNode.itemId }) - .then(aLivemark => { - let placesNode = aPopup._placesNode; - if (!placesNode.containerOpen) - return; - - if (aLivemark.status != Ci.mozILivemark.STATUS_LOADING) - this._setLivemarkStatusMenuItem(aPopup, aLivemark.status); - this._cleanPopup(aPopup, - this._nativeView && aPopup.parentNode.hasAttribute("open")); - - let children = aLivemark.getNodesForContainer(placesNode); - for (let i = 0; i < children.length; i++) { - let child = children[i]; - this.nodeInserted(placesNode, child, i); - if (child.accessCount) - this._getDOMNodeForPlacesNode(child).setAttribute("visited", true); - else - this._getDOMNodeForPlacesNode(child).removeAttribute("visited"); - } - }, Cu.reportError); - }, - /** * Checks whether the popup associated with the provided element is open. * This method may be overridden by classes that extend this base class. @@ -822,12 +627,6 @@ PlacesViewBase.prototype = { hasMultipleURIs = numURINodes > 1; } - let isLiveMark = false; - if (this.controller.hasCachedLivemarkInfo(aPopup._placesNode)) { - hasMultipleURIs = true; - isLiveMark = true; - } - if (!hasMultipleURIs) { aPopup.setAttribute("singleitempopup", "true"); } else { @@ -858,15 +657,9 @@ PlacesViewBase.prototype = { if (typeof this.options.extraClasses.footer == "string") aPopup._endOptOpenAllInTabs.classList.add(this.options.extraClasses.footer); - if (isLiveMark) { - aPopup._endOptOpenAllInTabs.setAttribute("oncommand", - "PlacesUIUtils.openLiveMarkNodesInTabs(this.parentNode._placesNode, event, " + - "PlacesUIUtils.getViewForNode(this));"); - } else { - aPopup._endOptOpenAllInTabs.setAttribute("oncommand", - "PlacesUIUtils.openContainerNodeInTabs(this.parentNode._placesNode, event, " + - "PlacesUIUtils.getViewForNode(this));"); - } + aPopup._endOptOpenAllInTabs.setAttribute("oncommand", + "PlacesUIUtils.openContainerNodeInTabs(this.parentNode._placesNode, event, " + + "PlacesUIUtils.getViewForNode(this));"); aPopup._endOptOpenAllInTabs.setAttribute("onclick", "checkForMiddleClick(this, event); event.stopPropagation();"); aPopup._endOptOpenAllInTabs.setAttribute("label", @@ -1120,12 +913,6 @@ PlacesToolbar.prototype = { button.setAttribute("query", "true"); if (PlacesUtils.nodeIsTagQuery(aChild)) button.setAttribute("tagContainer", "true"); - } else if (PlacesUtils.nodeIsFolder(aChild)) { - PlacesUtils.livemarks.getLivemark({ id: aChild.itemId }) - .then(aLivemark => { - button.setAttribute("livemark", "true"); - this.controller.cacheLivemarkInfo(aChild, aLivemark); - }, () => undefined); } let popup = document.createXULElement("menupopup"); @@ -1453,18 +1240,7 @@ PlacesToolbar.prototype = { if (elt.localName == "menupopup") elt = elt.parentNode; - if (elt.parentNode == this._rootElt) { // Node is on the toolbar. - // All livemarks have a feedURI, so use it as our indicator. - if (aAnno == PlacesUtils.LMANNO_FEEDURI) { - elt.setAttribute("livemark", true); - - PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) - .then(aLivemark => { - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - this.invalidateContainer(aPlacesNode); - }, Cu.reportError); - } - } else { + if (elt.parentNode != this._rootElt) { // Node is on the toolbar. // Node is in a submenu. PlacesViewBase.prototype.nodeAnnotationChanged.apply(this, arguments); } @@ -1548,7 +1324,7 @@ PlacesToolbar.prototype = { let eltRect = elt.getBoundingClientRect(); let eltIndex = Array.prototype.indexOf.call(this._rootElt.children, elt); if (PlacesUtils.nodeIsFolder(elt._placesNode) && - !PlacesUIUtils.isFolderReadOnly(elt._placesNode, this)) { + !PlacesUIUtils.isFolderReadOnly(elt._placesNode)) { // This is a folder. // If we are in the middle of it, drop inside it. // Otherwise, drop before it, with regards to RTL mode. @@ -1864,15 +1640,11 @@ PlacesToolbar.prototype = { let popup = aEvent.target; let placesNode = popup._placesNode; // Avoid handling popuphidden of inner views - if (placesNode && PlacesUIUtils.getViewForNode(popup) == this) { - // UI performance: folder queries are cheap, keep the resultnode open - // so we don't rebuild its contents whenever the popup is reopened. - // Though, we want to always close feed containers so their expiration - // status will be checked at next opening. - if (!PlacesUtils.nodeIsFolder(placesNode) || - this.controller.hasCachedLivemarkInfo(placesNode)) { - placesNode.containerOpen = false; - } + if (placesNode && PlacesUIUtils.getViewForNode(popup) == this && + // UI performance: folder queries are cheap, keep the resultnode open + // so we don't rebuild its contents whenever the popup is reopened. + !PlacesUtils.nodeIsFolder(placesNode)) { + placesNode.containerOpen = false; } let parent = popup.parentNode; @@ -1967,11 +1739,9 @@ PlacesMenu.prototype = { // UI performance: folder queries are cheap, keep the resultnode open // so we don't rebuild its contents whenever the popup is reopened. - // Though, we want to always close feed containers so their expiration - // status will be checked at next opening. - if (!PlacesUtils.nodeIsFolder(placesNode) || - this.controller.hasCachedLivemarkInfo(placesNode)) + if (!PlacesUtils.nodeIsFolder(placesNode)) { placesNode.containerOpen = false; + } // The autoopened attribute is set for folders which have been // automatically opened when dragged over. Turn off this attribute @@ -2023,12 +1793,6 @@ PlacesPanelMenuView.prototype = { button.setAttribute("query", "true"); if (PlacesUtils.nodeIsTagQuery(aChild)) button.setAttribute("tagContainer", "true"); - } else if (PlacesUtils.nodeIsFolder(aChild)) { - PlacesUtils.livemarks.getLivemark({ id: aChild.itemId }) - .then(aLivemark => { - button.setAttribute("livemark", "true"); - this.controller.cacheLivemarkInfo(aChild, aLivemark); - }, () => undefined); } } else if (PlacesUtils.nodeIsURI(aChild)) { button.setAttribute("scheme", @@ -2078,27 +1842,7 @@ PlacesPanelMenuView.prototype = { this._rootElt.insertBefore(elt, this._rootElt.children[aNewIndex]); }, - nodeAnnotationChanged: - function PAMV_nodeAnnotationChanged(aPlacesNode, aAnno) { - let elt = this._getDOMNodeForPlacesNode(aPlacesNode); - // There's no UI representation for the root node. - if (elt == this._rootElt) - return; - - if (elt.parentNode != this._rootElt) - return; - - // All livemarks have a feedURI, so use it as our indicator. - if (aAnno == PlacesUtils.LMANNO_FEEDURI) { - elt.setAttribute("livemark", true); - - PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) - .then(aLivemark => { - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - this.invalidateContainer(aPlacesNode); - }, Cu.reportError); - } - }, + nodeAnnotationChanged() {}, nodeTitleChanged: function PAMV_nodeTitleChanged(aPlacesNode, aNewTitle) { let elt = this._getDOMNodeForPlacesNode(aPlacesNode); @@ -2286,15 +2030,11 @@ this.PlacesPanelview = class extends PlacesViewBase { let panelview = event.originalTarget; let placesNode = panelview._placesNode; // Avoid handling ViewHiding of inner views - if (placesNode && PlacesUIUtils.getViewForNode(panelview) == this) { - // UI performance: folder queries are cheap, keep the resultnode open - // so we don't rebuild its contents whenever the popup is reopened. - // Though, we want to always close feed containers so their expiration - // status will be checked at next opening. - if (!PlacesUtils.nodeIsFolder(placesNode) || - this.controller.hasCachedLivemarkInfo(placesNode)) { - placesNode.containerOpen = false; - } + if (placesNode && PlacesUIUtils.getViewForNode(panelview) == this && + // UI performance: folder queries are cheap, keep the resultnode open + // so we don't rebuild its contents whenever the popup is reopened. + !PlacesUtils.nodeIsFolder(placesNode)) { + placesNode.containerOpen = false; } } diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js index 5e3d60167229..491915eb8632 100644 --- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -189,7 +189,7 @@ PlacesController.prototype = { let selectedNode = this._view.selectedNode; return selectedNode && PlacesUtils.nodeIsFolder(selectedNode) && - !PlacesUIUtils.isFolderReadOnly(selectedNode, this._view) && + !PlacesUIUtils.isFolderReadOnly(selectedNode) && this._view.result.sortingMode == Ci.nsINavHistoryQueryOptions.SORT_BY_NONE; } @@ -308,7 +308,7 @@ PlacesController.prototype = { if (nodes[i] == root) return false; - if (!PlacesUIUtils.canUserRemove(nodes[i], this._view)) + if (!PlacesUIUtils.canUserRemove(nodes[i])) return false; } } @@ -1242,7 +1242,7 @@ PlacesController.prototype = { // Allow dropping into Tag containers and editable folders. return !PlacesUtils.nodeIsTagQuery(container) && (!PlacesUtils.nodeIsFolder(container) || - PlacesUIUtils.isFolderReadOnly(container, this._view)); + PlacesUIUtils.isFolderReadOnly(container)); }, /** @@ -1262,7 +1262,7 @@ PlacesController.prototype = { let parentNode = node.parent; return parentNode != null && PlacesUtils.nodeIsFolder(parentNode) && - !PlacesUIUtils.isFolderReadOnly(parentNode, this._view) && + !PlacesUIUtils.isFolderReadOnly(parentNode) && !PlacesUtils.nodeIsTagQuery(parentNode); }, }; diff --git a/browser/components/places/content/menu.xml b/browser/components/places/content/menu.xml index 441765c92fb9..be83d0bac1a2 100644 --- a/browser/components/places/content/menu.xml +++ b/browser/components/places/content/menu.xml @@ -110,7 +110,7 @@ let tagName = PlacesUtils.nodeIsTagQuery(elt._placesNode) ? elt._placesNode.title : null; if ((PlacesUtils.nodeIsFolder(elt._placesNode) && - !PlacesUIUtils.isFolderReadOnly(elt._placesNode, this._rootView)) || + !PlacesUIUtils.isFolderReadOnly(elt._placesNode)) || PlacesUtils.nodeIsTagQuery(elt._placesNode)) { // This is a folder or a tag container. if (eventY - eltY < eltHeight * 0.20) { diff --git a/browser/components/places/content/treeView.js b/browser/components/places/content/treeView.js index 26f6f1d45967..fc526b434bc9 100644 --- a/browser/components/places/content/treeView.js +++ b/browser/components/places/content/treeView.js @@ -107,10 +107,6 @@ PlacesTreeView.prototype = { * @return true if aContainer is a plain container, false otherwise. */ _isPlainContainer: function PTV__isPlainContainer(aContainer) { - // Livemarks are always plain containers. - if (this._controller.hasCachedLivemarkInfo(aContainer)) - return true; - // We don't know enough about non-query containers. if (!(aContainer instanceof Ci.nsINavHistoryQueryResultNode)) return false; @@ -343,8 +339,7 @@ PlacesTreeView.prototype = { // Recursively do containers. if (!this._flatList && - curChild instanceof Ci.nsINavHistoryContainerResultNode && - !this._controller.hasCachedLivemarkInfo(curChild)) { + curChild instanceof Ci.nsINavHistoryContainerResultNode) { let uri = curChild.uri; let isopen = false; @@ -831,22 +826,6 @@ PlacesTreeView.prototype = { } }, - _populateLivemarkContainer: function PTV__populateLivemarkContainer(aNode) { - PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }) - .then(aLivemark => { - let placesNode = aNode; - // Need to check containerOpen since getLivemark is async. - if (!placesNode.containerOpen) - return; - - let children = aLivemark.getNodesForContainer(placesNode); - for (let i = 0; i < children.length; i++) { - let child = children[i]; - this.nodeInserted(placesNode, child, i); - } - }, Cu.reportError); - }, - nodeTitleChanged: function PTV_nodeTitleChanged(aNode, aNewTitle) { this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE); }, @@ -870,19 +849,6 @@ PlacesTreeView.prototype = { itemId: aNode.itemId, time: aOldVisitDate})); this._nodeDetails.set(makeNodeDetailsKey(aNode), aNode); - if (aNode.parent && this._controller.hasCachedLivemarkInfo(aNode.parent)) { - // Find the node in the parent. - let parentRow = this._flatList ? 0 : this._getRowForNode(aNode.parent); - for (let i = parentRow; i < this._rows.length; i++) { - let child = this.nodeForTreeIndex(i); - if (child.uri == aNode.uri) { - this._cellProperties.delete(child); - this._invalidateCellValue(child, this.COLUMN_TYPE_TITLE); - break; - } - } - return; - } this._invalidateCellValue(aNode, this.COLUMN_TYPE_DATE); this._invalidateCellValue(aNode, this.COLUMN_TYPE_VISITCOUNT); @@ -894,18 +860,7 @@ PlacesTreeView.prototype = { nodeKeywordChanged(aNode, aNewKeyword) {}, - nodeAnnotationChanged: function PTV_nodeAnnotationChanged(aNode, aAnno) { - if (aAnno == PlacesUtils.LMANNO_FEEDURI) { - PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }) - .then(aLivemark => { - this._controller.cacheLivemarkInfo(aNode, aLivemark); - let properties = this._cellProperties.get(aNode); - this._cellProperties.set(aNode, properties += " livemark"); - // The livemark attribute is set as a cell property on the title cell. - this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE); - }, Cu.reportError); - } - }, + nodeAnnotationChanged() {}, nodeDateAddedChanged: function PTV_nodeDateAddedChanged(aNode, aNewValue) { this._invalidateCellValue(aNode, this.COLUMN_TYPE_DATEADDED); @@ -919,32 +874,6 @@ PlacesTreeView.prototype = { containerStateChanged: function PTV_containerStateChanged(aNode, aOldState, aNewState) { this.invalidateContainer(aNode); - - if (PlacesUtils.nodeIsFolder(aNode) || - (this._flatList && aNode == this._rootNode)) { - let queryOptions = PlacesUtils.asQuery(this._rootNode).queryOptions; - if (queryOptions.excludeItems) { - return; - } - if (aNode.itemId != -1) { // run when there's a valid node id - PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }) - .then(aLivemark => { - let shouldInvalidate = - !this._controller.hasCachedLivemarkInfo(aNode); - this._controller.cacheLivemarkInfo(aNode, aLivemark); - if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED) { - aLivemark.registerForUpdates(aNode, this); - // Prioritize the current livemark. - aLivemark.reload(); - PlacesUtils.livemarks.reloadLivemarks(); - if (shouldInvalidate) - this.invalidateContainer(aNode); - } else { - aLivemark.unregisterForUpdates(aNode); - } - }, () => undefined); - } - } }, invalidateContainer: function PTV_invalidateContainer(aContainer) { @@ -1065,13 +994,6 @@ PlacesTreeView.prototype = { } } - if (this._controller.hasCachedLivemarkInfo(aContainer)) { - let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions; - if (!queryOptions.excludeItems) { - this._populateLivemarkContainer(aContainer); - } - } - this._tree.endUpdateBatch(); // Restore selection. @@ -1248,22 +1170,6 @@ PlacesTreeView.prototype = { properties += " dayContainer"; else if (PlacesUtils.nodeIsHost(node)) properties += " hostContainer"; - } else if (nodeType == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER || - nodeType == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT) { - if (itemId != -1) { - if (this._controller.hasCachedLivemarkInfo(node)) { - properties += " livemark"; - } else { - PlacesUtils.livemarks.getLivemark({ id: itemId }) - .then(aLivemark => { - this._controller.cacheLivemarkInfo(node, aLivemark); - let livemarkProps = this._cellProperties.get(node); - this._cellProperties.set(node, livemarkProps += " livemark"); - // The livemark attribute is set as a cell property on the title cell. - this._invalidateCellValue(node, this.COLUMN_TYPE_TITLE); - }, () => undefined); - } - } } if (itemId == -1) { @@ -1289,13 +1195,6 @@ PlacesTreeView.prototype = { properties += " separator"; else if (PlacesUtils.nodeIsURI(node)) { properties += " " + PlacesUIUtils.guessUrlSchemeForUI(node.uri); - - if (this._controller.hasCachedLivemarkInfo(node.parent)) { - properties += " livemarkItem"; - if (node.accessCount) { - properties += " visited"; - } - } } this._cellProperties.set(node, properties); @@ -1338,14 +1237,8 @@ PlacesTreeView.prototype = { if (this._flatList) return true; - let node = this._rows[aRow]; - if (this._controller.hasCachedLivemarkInfo(node)) { - let queryOptions = PlacesUtils.asQuery(this._result.root).queryOptions; - return queryOptions.excludeItems; - } - // All containers are listed in the rows array. - return !node.hasChildren; + return !this._rows[aRow].hasChildren; }, isSeparator: function PTV_isSeparator(aRow) { @@ -1590,18 +1483,15 @@ PlacesTreeView.prototype = { return; } - // Persist containers open status, but never persist livemarks. - if (!this._controller.hasCachedLivemarkInfo(node)) { - let uri = node.uri; + let uri = node.uri; - if (uri) { - let docURI = document.documentURI; + if (uri) { + let docURI = document.documentURI; - if (node.containerOpen) { - Services.xulStore.removeValue(docURI, uri, "open"); - } else { - Services.xulStore.setValue(docURI, uri, "open", "true"); - } + if (node.containerOpen) { + Services.xulStore.removeValue(docURI, uri, "open"); + } else { + Services.xulStore.setValue(docURI, uri, "open", "true"); } } @@ -1716,9 +1606,8 @@ PlacesTreeView.prototype = { } let itemGuid = node.bookmarkGuid; - // Only bookmark-nodes are editable. Fortunately, this checks also takes - // care of livemark children. - if (itemGuid == "") + // Only bookmark-nodes are editable. + if (!itemGuid) return false; // The following items are also not editable, even though they are bookmark diff --git a/browser/components/preferences/in-content/findInPage.js b/browser/components/preferences/in-content/findInPage.js index c3a336fd0220..18e93e8432dc 100644 --- a/browser/components/preferences/in-content/findInPage.js +++ b/browser/components/preferences/in-content/findInPage.js @@ -547,7 +547,9 @@ var gSearchResultsPane = { removeAllSearchTooltips() { for (let anchorNode of this.listSearchTooltips) { anchorNode.parentElement.classList.remove("search-tooltip-parent"); - anchorNode.tooltipNode.remove(); + if (anchorNode.tooltipNode) { + anchorNode.tooltipNode.remove(); + } anchorNode.tooltipNode = null; } this.listSearchTooltips.clear(); diff --git a/browser/components/urlbar/UrlbarInput.jsm b/browser/components/urlbar/UrlbarInput.jsm index bc9f526ffe92..9a8a7d98d2ae 100644 --- a/browser/components/urlbar/UrlbarInput.jsm +++ b/browser/components/urlbar/UrlbarInput.jsm @@ -230,7 +230,16 @@ class UrlbarInput { * @param {UrlbarMatch} result The result that was selected. */ resultSelected(event, result) { - // Set the input value to the target url. + this.setValueFromResult(result); + this.controller.resultSelected(event, result); + } + + /** + * Called by the view when moving through results with the keyboard. + * + * @param {UrlbarMatch} result The result that was selected. + */ + setValueFromResult(result) { let val = result.url; let uri; try { @@ -240,8 +249,6 @@ class UrlbarInput { val = this.window.losslessDecodeURI(uri); } this.value = val; - - this.controller.resultSelected(event, result); } // Getters and Setters below. diff --git a/browser/components/urlbar/UrlbarPrefs.jsm b/browser/components/urlbar/UrlbarPrefs.jsm index 6e65373f1b0e..32865b16c7e7 100644 --- a/browser/components/urlbar/UrlbarPrefs.jsm +++ b/browser/components/urlbar/UrlbarPrefs.jsm @@ -68,10 +68,6 @@ const PREF_URLBAR_DEFAULTS = new Map([ // INSERTMETHOD values. ["insertMethod", UrlbarUtils.INSERTMETHOD.MERGE_RELATED], - // Controls how URLs are matched against the user's search string. See - // mozIPlacesAutoComplete. - ["matchBehavior", Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY_ANYWHERE], - // Controls the composition of search results. ["matchBuckets", "suggestion:4,general:Infinity"], @@ -311,16 +307,6 @@ class Preferences { } return val; } - case "matchBehavior": { - // Validate matchBehavior; default to MATCH_BOUNDARY_ANYWHERE. - let val = this._readPref(pref); - if (![Ci.mozIPlacesAutoComplete.MATCH_ANYWHERE, - Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY, - Ci.mozIPlacesAutoComplete.MATCH_BEGINNING].includes(val)) { - val = Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY_ANYWHERE; - } - return val; - } } return this._readPref(pref); } diff --git a/browser/locales/en-US/chrome/browser/places/places.properties b/browser/locales/en-US/chrome/browser/places/places.properties index 1b1a3ebf996f..a7c7ec1a2185 100644 --- a/browser/locales/en-US/chrome/browser/places/places.properties +++ b/browser/locales/en-US/chrome/browser/places/places.properties @@ -17,11 +17,6 @@ bookmarksRestoreFilterName=JSON bookmarksRestoreFormatError=Unsupported file type. bookmarksRestoreParseError=Unable to process the backup file. -bookmarksLivemarkLoading=Live Bookmark loading… -bookmarksLivemarkFailed=Live Bookmark feed failed to load. - -menuOpenLivemarkOrigin.label=Open “%S” - sortByName=Sort ‘%S’ by Name sortByNameGeneric=Sort by Name # LOCALIZATION NOTE (view.sortBy.1.name.label): sortBy properties are versioned. diff --git a/browser/themes/linux/jar.mn b/browser/themes/linux/jar.mn index 45f959d2ab92..4d7cfd987180 100644 --- a/browser/themes/linux/jar.mn +++ b/browser/themes/linux/jar.mn @@ -26,7 +26,6 @@ browser.jar: skin/classic/browser/notification-icons/geo.svg (notification-icons/geo.svg) skin/classic/browser/places/allBookmarks.png (places/allBookmarks.png) skin/classic/browser/places/editBookmark.css (places/editBookmark.css) - skin/classic/browser/places/livemark-item.png (places/livemark-item.png) * skin/classic/browser/places/sidebar.css (places/sidebar.css) skin/classic/browser/places/organizer.css (places/organizer.css) skin/classic/browser/places/organizer.xml (places/organizer.xml) diff --git a/browser/themes/linux/places/livemark-item.png b/browser/themes/linux/places/livemark-item.png deleted file mode 100644 index 07342bea3bf6..000000000000 Binary files a/browser/themes/linux/places/livemark-item.png and /dev/null differ diff --git a/browser/themes/osx/browser.css b/browser/themes/osx/browser.css index 435150a025a7..c534ad4cad20 100644 --- a/browser/themes/osx/browser.css +++ b/browser/themes/osx/browser.css @@ -189,9 +189,7 @@ } /* Workaround for native menubar inheritance */ -.openintabs-menuitem, -.openlivemarksite-menuitem, -.livemarkstatus-menuitem { +.openintabs-menuitem { list-style-image: none; } diff --git a/browser/themes/osx/jar.mn b/browser/themes/osx/jar.mn index b330f4dbd456..71e840323465 100644 --- a/browser/themes/osx/jar.mn +++ b/browser/themes/osx/jar.mn @@ -35,7 +35,6 @@ browser.jar: skin/classic/browser/places/toolbar.png (places/toolbar.png) skin/classic/browser/places/toolbarDropMarker.png (places/toolbarDropMarker.png) skin/classic/browser/places/editBookmark.css (places/editBookmark.css) - skin/classic/browser/places/livemark-item.png (places/livemark-item.png) skin/classic/browser/preferences/alwaysAsk.png (preferences/alwaysAsk.png) skin/classic/browser/preferences/application.png (preferences/application.png) skin/classic/browser/preferences/saveFile.png (preferences/saveFile.png) diff --git a/browser/themes/osx/places/livemark-item.png b/browser/themes/osx/places/livemark-item.png deleted file mode 100644 index 07342bea3bf6..000000000000 Binary files a/browser/themes/osx/places/livemark-item.png and /dev/null differ diff --git a/browser/themes/shared/jar.inc.mn b/browser/themes/shared/jar.inc.mn index 9b75384846a8..c6a2a1f8bf0a 100644 --- a/browser/themes/shared/jar.inc.mn +++ b/browser/themes/shared/jar.inc.mn @@ -235,7 +235,6 @@ skin/classic/browser/places/bookmarksMenu.svg (../shared/places/bookmarksMenu.svg) skin/classic/browser/places/bookmarksToolbar.svg (../shared/places/bookmarksToolbar.svg) skin/classic/browser/places/folder.svg (../shared/places/folder.svg) - skin/classic/browser/places/folder-live.svg (../shared/places/folder-live.svg) skin/classic/browser/places/folder-smart.svg (../shared/places/folder-smart.svg) skin/classic/browser/places/history.svg (../shared/places/history.svg) skin/classic/browser/places/tag.svg (../shared/places/tag.svg) diff --git a/browser/themes/shared/places/folder-live.svg b/browser/themes/shared/places/folder-live.svg deleted file mode 100644 index 532227b5a2d6..000000000000 --- a/browser/themes/shared/places/folder-live.svg +++ /dev/null @@ -1,6 +0,0 @@ - - - - diff --git a/browser/themes/shared/places/tree-icons.css b/browser/themes/shared/places/tree-icons.css index 8d386da7496d..f74bf1aaa8f8 100644 --- a/browser/themes/shared/places/tree-icons.css +++ b/browser/themes/shared/places/tree-icons.css @@ -15,15 +15,6 @@ treechildren::-moz-tree-image(title) { height: 16px; } -treechildren::-moz-tree-image(title, livemarkItem) { - list-style-image: url("chrome://browser/skin/places/livemark-item.png"); - -moz-image-region: rect(0px, 16px, 16px, 0px); -} - -treechildren::-moz-tree-image(title, livemarkItem, visited) { - -moz-image-region: rect(0px, 32px, 16px, 16px); -} - treechildren::-moz-tree-image(title, container), treechildren::-moz-tree-image(title, open) { list-style-image: url("chrome://browser/skin/places/folder.svg"); @@ -36,10 +27,6 @@ treechildren::-moz-tree-image(title, separator) { margin: 0; } -treechildren::-moz-tree-image(container, livemark) { - list-style-image: url("chrome://browser/skin/places/folder-live.svg"); -} - treechildren::-moz-tree-image(container, queryFolder_toolbar_____) { list-style-image: url("chrome://browser/skin/places/bookmarksToolbar.svg"); } diff --git a/browser/themes/shared/toolbarbutton-icons.inc.css b/browser/themes/shared/toolbarbutton-icons.inc.css index 41c2f8b92912..a0d4eff9108d 100644 --- a/browser/themes/shared/toolbarbutton-icons.inc.css +++ b/browser/themes/shared/toolbarbutton-icons.inc.css @@ -514,19 +514,6 @@ toolbar[brighttext] { list-style-image: url("chrome://browser/skin/places/folder.svg"); } -.bookmark-item[container][livemark] { - list-style-image: url("chrome://browser/skin/places/folder-live.svg"); -} - -.bookmark-item[container][livemark] .bookmark-item { - list-style-image: url("chrome://browser/skin/places/livemark-item.png"); - -moz-image-region: rect(0px, 16px, 16px, 0px); -} - -.bookmark-item[container][livemark] .bookmark-item[visited] { - -moz-image-region: rect(0px, 32px, 16px, 16px); -} - .bookmark-item[container][query] { list-style-image: url("chrome://browser/skin/places/folder-smart.svg"); } diff --git a/browser/themes/windows/jar.mn b/browser/themes/windows/jar.mn index 4ad2c7025216..26f9ed4b3559 100644 --- a/browser/themes/windows/jar.mn +++ b/browser/themes/windows/jar.mn @@ -30,7 +30,6 @@ browser.jar: skin/classic/browser/places/editBookmark.css (places/editBookmark.css) skin/classic/browser/places/libraryToolbar.png (places/libraryToolbar.png) skin/classic/browser/places/allBookmarks.png (places/allBookmarks.png) - skin/classic/browser/places/livemark-item.png (places/livemark-item.png) skin/classic/browser/preferences/alwaysAsk.png (preferences/alwaysAsk.png) skin/classic/browser/preferences/application.png (preferences/application.png) skin/classic/browser/preferences/saveFile.png (preferences/saveFile.png) diff --git a/browser/themes/windows/places/livemark-item.png b/browser/themes/windows/places/livemark-item.png deleted file mode 100644 index 07342bea3bf6..000000000000 Binary files a/browser/themes/windows/places/livemark-item.png and /dev/null differ diff --git a/caps/OriginAttributes.cpp b/caps/OriginAttributes.cpp index 7ec9bc66b925..8590b407d2f7 100644 --- a/caps/OriginAttributes.cpp +++ b/caps/OriginAttributes.cpp @@ -19,6 +19,7 @@ using dom::URLParams; bool OriginAttributes::sFirstPartyIsolation = false; bool OriginAttributes::sRestrictedOpenerAccess = false; +bool OriginAttributes::sBlockPostMessageForFPI = false; void OriginAttributes::InitPrefs() @@ -31,6 +32,8 @@ OriginAttributes::InitPrefs() "privacy.firstparty.isolate"); Preferences::AddBoolVarCache(&sRestrictedOpenerAccess, "privacy.firstparty.isolate.restrict_opener_access"); + Preferences::AddBoolVarCache(&sBlockPostMessageForFPI, + "privacy.firstparty.isolate.block_post_message"); } } diff --git a/caps/OriginAttributes.h b/caps/OriginAttributes.h index b0ca8bf2fa19..be257cf919e9 100644 --- a/caps/OriginAttributes.h +++ b/caps/OriginAttributes.h @@ -61,6 +61,14 @@ public: return !(*this == aOther); } + MOZ_MUST_USE bool EqualsIgnoringFPD(const OriginAttributes& aOther) const + { + return mAppId == aOther.mAppId && + mInIsolatedMozBrowser == aOther.mInIsolatedMozBrowser && + mUserContextId == aOther.mUserContextId && + mPrivateBrowsingId == aOther.mPrivateBrowsingId; + } + // Serializes/Deserializes non-default values into the suffix format, i.e. // |!key1=value1&key2=value2|. If there are no non-default attributes, this // returns an empty string. @@ -96,6 +104,13 @@ public: return !sFirstPartyIsolation || sRestrictedOpenerAccess; } + // Check whether we block the postMessage across different FPDs when the + // targetOrigin is '*'. + static inline MOZ_MUST_USE bool IsBlockPostMessageForFPI() + { + return sFirstPartyIsolation && sBlockPostMessageForFPI; + } + // returns true if the originAttributes suffix has mPrivateBrowsingId value // different than 0. static bool IsPrivateBrowsing(const nsACString& aOrigin); @@ -105,6 +120,7 @@ public: private: static bool sFirstPartyIsolation; static bool sRestrictedOpenerAccess; + static bool sBlockPostMessageForFPI; }; class OriginAttributesPattern : public dom::OriginAttributesPatternDictionary diff --git a/devtools/client/inspector/changes/ChangesView.js b/devtools/client/inspector/changes/ChangesView.js index f3212c9fcf60..5d6cca0719a6 100644 --- a/devtools/client/inspector/changes/ChangesView.js +++ b/devtools/client/inspector/changes/ChangesView.js @@ -17,7 +17,8 @@ const { } = require("./actions/changes"); class ChangesView { - constructor(inspector) { + constructor(inspector, window) { + this.document = window.document; this.inspector = inspector; this.store = this.inspector.store; this.toolbox = this.inspector.toolbox; @@ -44,9 +45,7 @@ class ChangesView { store: this.store, }, changesApp); - // TODO: save store and restore/replay on refresh. - // Bug 1478439 - https://bugzilla.mozilla.org/show_bug.cgi?id=1478439 - this.inspector.target.once("will-navigate", this.destroy); + this.inspector.target.on("will-navigate", this.onClearChanges); // Sync the store to the changes stored on the server. The // syncChangesToServer() method is async, but we don't await it since @@ -86,6 +85,7 @@ class ChangesView { this.changesFront.off("clear-changes", this.onClearChanges); this.changesFront = null; + this.document = null; this.inspector = null; this.store = null; this.toolbox = null; diff --git a/devtools/client/inspector/changes/actions/changes.js b/devtools/client/inspector/changes/actions/changes.js index c2ac8bad7901..d1e4bbbaa912 100644 --- a/devtools/client/inspector/changes/actions/changes.js +++ b/devtools/client/inspector/changes/actions/changes.js @@ -17,10 +17,10 @@ module.exports = { }; }, - trackChange(data) { + trackChange(change) { return { type: TRACK_CHANGE, - data, + change, }; }, diff --git a/devtools/client/inspector/changes/components/CSSDeclaration.js b/devtools/client/inspector/changes/components/CSSDeclaration.js new file mode 100644 index 000000000000..f28c7eee2700 --- /dev/null +++ b/devtools/client/inspector/changes/components/CSSDeclaration.js @@ -0,0 +1,38 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ + +"use strict"; + +const { PureComponent } = require("devtools/client/shared/vendor/react"); +const dom = require("devtools/client/shared/vendor/react-dom-factories"); +const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); + +class CSSDeclaration extends PureComponent { + static get propTypes() { + return { + property: PropTypes.string.isRequired, + value: PropTypes.string.isRequired, + className: PropTypes.string, + }; + } + + static get defaultProps() { + return { + className: "", + }; + } + + render() { + const { property, value, className } = this.props; + + return dom.div({ className }, + dom.span({ className: "declaration-name theme-fg-color5"}, property), + ":", + dom.span({ className: "declaration-value theme-fg-color1"}, value), + ";" + ); + } +} + +module.exports = CSSDeclaration; diff --git a/devtools/client/inspector/changes/components/ChangesApp.js b/devtools/client/inspector/changes/components/ChangesApp.js index f6e8834c1871..1ae95abaa536 100644 --- a/devtools/client/inspector/changes/components/ChangesApp.js +++ b/devtools/client/inspector/changes/components/ChangesApp.js @@ -4,73 +4,114 @@ "use strict"; -const { PureComponent } = require("devtools/client/shared/vendor/react"); +const { createFactory, PureComponent } = require("devtools/client/shared/vendor/react"); const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); const { connect } = require("devtools/client/shared/vendor/react-redux"); +const CSSDeclaration = createFactory(require("./CSSDeclaration")); + class ChangesApp extends PureComponent { static get propTypes() { return { + // Redux state slice assigned to Track Changes feature; passed as prop by connect() changes: PropTypes.object.isRequired, }; } - renderMutations(remove = {}, add = {}) { - const removals = Object.entries(remove).map(([prop, value]) => { - return dom.div( - { className: "line diff-remove"}, - `${prop}: ${value};` - ); + constructor(props) { + super(props); + // In the Redux store, all rules exist in a collection at the same level of nesting. + // Parent rules come before child rules. Parent/child dependencies are set + // via parameters in each rule pointing to the corresponding rule ids. + // + // To render rules, we traverse the descendant rule tree and render each child rule + // found. This means we get into situations where we can render the same rule multiple + // times: once as a child of its parent and once standalone. + // + // By keeping a log of rules previously rendered we prevent needless multi-rendering. + this.renderedRules = []; + } + + renderDeclarations(remove = {}, add = {}) { + const removals = Object.entries(remove).map(([property, value]) => { + return CSSDeclaration({ className: "level diff-remove", property, value }); }); - const additions = Object.entries(add).map(([prop, value]) => { - return dom.div( - { className: "line diff-add"}, - `${prop}: ${value};` - ); + const additions = Object.entries(add).map(([property, value]) => { + return CSSDeclaration({ className: "level diff-add", property, value }); }); return [removals, additions]; } - renderSelectors(selectors = {}) { - return Object.keys(selectors).map(sel => { - return dom.details( - { className: "selector", open: true }, - dom.summary( - { - title: sel, - }, - sel), - this.renderMutations(selectors[sel].remove, selectors[sel].add) - ); - }); + renderRule(ruleId, rule, rules) { + const selector = rule.selector; + + if (this.renderedRules.includes(ruleId)) { + return null; + } + + // Mark this rule as rendered so we don't render it again. + this.renderedRules.push(ruleId); + + return dom.div( + { + className: "rule", + }, + dom.div( + { + className: "level selector", + title: selector, + }, + selector, + dom.span({ className: "bracket-open" }, "{") + ), + // Render any nested child rules if they are present. + rule.children.length > 0 && rule.children.map(childRuleId => { + return this.renderRule(childRuleId, rules[childRuleId], rules); + }), + // Render any changed CSS declarations. + this.renderDeclarations(rule.remove, rule.add), + dom.span({ className: "level bracket-close" }, "}") + ); } - renderDiff(diff = {}) { - // Render groups of style sources: stylesheets, embedded styles and inline styles - return Object.keys(diff).map(href => { + renderDiff(changes = {}) { + // Render groups of style sources: stylesheets and element style attributes. + return Object.entries(changes).map(([sourceId, source]) => { + const href = source.href || `inline stylesheet #${source.index}`; + const rules = source.rules; + return dom.details( - { className: "source", open: true }, + { + className: "source devtools-monospace", + open: true, + }, dom.summary( { + className: "href", title: href, }, href), - // Render groups of selectors - this.renderSelectors(diff[href]) + // Render changed rules within this source. + Object.entries(rules).map(([ruleId, rule]) => { + return this.renderRule(ruleId, rule, rules); + }) ); }); } render() { + // Reset log of rendered rules. + this.renderedRules = []; + return dom.div( { className: "theme-sidebar inspector-tabpanel", id: "sidebar-panel-changes", }, - this.renderDiff(this.props.changes.diff) + this.renderDiff(this.props.changes) ); } } diff --git a/devtools/client/inspector/changes/components/moz.build b/devtools/client/inspector/changes/components/moz.build index e2a9a176c0ba..d7d2d06dd042 100644 --- a/devtools/client/inspector/changes/components/moz.build +++ b/devtools/client/inspector/changes/components/moz.build @@ -6,4 +6,5 @@ DevToolsModules( 'ChangesApp.js', + 'CSSDeclaration.js', ) diff --git a/devtools/client/inspector/changes/moz.build b/devtools/client/inspector/changes/moz.build index be35ad25e2b4..e805e4af65a9 100644 --- a/devtools/client/inspector/changes/moz.build +++ b/devtools/client/inspector/changes/moz.build @@ -8,9 +8,12 @@ DIRS += [ 'actions', 'components', 'reducers', + 'utils', ] DevToolsModules( 'ChangesManager.js', 'ChangesView.js', ) + +BROWSER_CHROME_MANIFESTS += ['test/browser.ini'] diff --git a/devtools/client/inspector/changes/reducers/changes.js b/devtools/client/inspector/changes/reducers/changes.js index 936ba98b9bce..380bd69dd75e 100644 --- a/devtools/client/inspector/changes/reducers/changes.js +++ b/devtools/client/inspector/changes/reducers/changes.js @@ -4,113 +4,229 @@ "use strict"; +const { getSourceHash, getRuleHash } = require("../utils/changes-utils"); + const { RESET_CHANGES, TRACK_CHANGE, } = require("../actions/index"); -const INITIAL_STATE = { - /** - * Diff of changes grouped by stylesheet href, then by selector, then into add/remove - * objects with CSS property names and values for corresponding changes. - * - * Structure: - * - * diff = { - * "href": { - * "selector": { - * add: { - * "property": value - * ... // more properties - * }, - * remove: { - * "property": value - * ... - * } - * }, - * ... // more selectors - * } - * ... // more stylesheet hrefs - * } - */ - diff: {}, -}; +/** + * Return a deep clone of the given state object. + * + * @param {Object} state + * @return {Object} + */ +function cloneState(state = {}) { + return Object.entries(state).reduce((sources, [sourceId, source]) => { + sources[sourceId] = { + ...source, + rules: Object.entries(source.rules).reduce((rules, [ruleId, rule]) => { + rules[ruleId] = { + ...rule, + children: rule.children.slice(0), + add: { ...rule.add }, + remove: { ...rule.remove }, + }; + + return rules; + }, {}), + }; + + return sources; + }, {}); +} /** - * Mutate the given diff object with data about a new change. + * Given information about a CSS rule and its ancestor rules (@media, @supports, etc), + * create entries in the given rules collection for each rule and assign parent/child + * dependencies. + * + * @param {Object} ruleData + * Information about a CSS rule: + * { + * selector: {String} + * CSS selector text + * ancestors: {Array} + * Flattened CSS rule tree of the rule's ancestors with the root rule + * at the beginning of the array and the leaf rule at the end. + * ruleIndex: {Array} + * Indexes of each ancestor rule within its parent rule. + * } + * + * @param {Object} rules + * Collection of rules to be mutated. + * This is a reference to the corresponding `rules` object from the state. * - * @param {Object} diff - * Diff object from the store. - * @param {Object} change - * Data about the change: which property was added or removed, on which selector, - * in which stylesheet or whether the source is an element's inline style. * @return {Object} - * Mutated diff object. + * Entry for the CSS rule created the given collection of rules. */ -function updateDiff(diff = {}, change = {}) { - // Ensure expected diff structure exists - diff[change.href] = diff[change.href] || {}; - diff[change.href][change.selector] = diff[change.href][change.selector] || {}; - diff[change.href][change.selector].add = diff[change.href][change.selector].add || {}; - diff[change.href][change.selector].remove = diff[change.href][change.selector].remove - || {}; +function createRule(ruleData, rules) { + // Append the rule data to the flattened CSS rule tree with its ancestors. + const ruleAncestry = [...ruleData.ancestors, { ...ruleData }]; - // Reference to the diff data for this stylesheet/selector pair. - const ref = diff[change.href][change.selector]; + return ruleAncestry + // First, generate a unique identifier for each rule. + .map((rule, index) => { + // Ensure each rule has ancestors excluding itself (expand the flattened rule tree). + rule.ancestors = ruleAncestry.slice(0, index); + // Ensure each rule has a selector text. + // For the purpose of displaying in the UI, we treat at-rules as selectors. + if (!rule.selector) { + rule.selector = + `${rule.typeName} ${(rule.conditionText || rule.name || rule.keyText)}`; + } - // Track the remove operation only if the property WAS NOT previously introduced by an - // add operation. This ensures that repeated changes of the same property show up as a - // single remove operation of the original value. - if (change.remove && change.remove.property && !ref.add[change.remove.property]) { - ref.remove[change.remove.property] = change.remove.value; - } + return getRuleHash(rule); + }) + // Then, create new entries in the rules collection and assign dependencies. + .map((ruleId, index, array) => { + const { selector } = ruleAncestry[index]; + const prevRuleId = array[index - 1]; + const nextRuleId = array[index + 1]; - if (change.add && change.add.property) { - ref.add[change.add.property] = change.add.value; - } + // Copy or create an entry for this rule. + rules[ruleId] = Object.assign({}, { selector, children: [] }, rules[ruleId]); - const propertyName = change.add && change.add.property || - change.remove && change.remove.property; + // The next ruleId is lower in the rule tree, therefore it's a child of this rule. + if (nextRuleId && !rules[ruleId].children.includes(nextRuleId)) { + rules[ruleId].children.push(nextRuleId); + } - // Remove information about operations on the property if they cancel each other out. - if (ref.add[propertyName] === ref.remove[propertyName]) { - delete ref.add[propertyName]; - delete ref.remove[propertyName]; + // The previous ruleId is higher in the rule tree, therefore it's the parent. + if (prevRuleId) { + rules[ruleId].parent = prevRuleId; + } - // Remove information about the selector if there are no changes to its declarations. - if (Object.keys(ref.add).length === 0 && Object.keys(ref.remove).length === 0) { - delete diff[change.href][change.selector]; - } - - // Remove information about the stylesheet if there are no changes to its rules. - if (Object.keys(diff[change.href]).length === 0) { - delete diff[change.href]; - } - } - - ref.tag = change.tag; - - return diff; + return rules[ruleId]; + }) + // Finally, return the last rule in the array which is the rule we set out to create. + .pop(); } +function removeRule(ruleId, rules) { + const rule = rules[ruleId]; + + // First, remove this rule's id from its parent's list of children + if (rule.parent && rules[rule.parent]) { + rules[rule.parent].children = rules[rule.parent].children.filter(childRuleId => { + return childRuleId !== ruleId; + }); + + // Remove the parent rule if it has no children left. + if (!rules[rule.parent].children.length) { + removeRule(rule.parent, rules); + } + } + + delete rules[ruleId]; +} + +/** + * Aggregated changes grouped by sources (stylesheet/element), which contain rules, + * which contain collections of added and removed CSS declarations. + * + * Structure: + * : { + * type: // "stylesheet" or "element" + * href: // Stylesheet or document URL + * rules: { + * : { + * selector: "" // String CSS selector or CSS at-rule text + * children: [] // Array of for child rules of this rule. + * parent: // of the parent rule + * add: { + * : // CSS declaration + * ... + * }, + * remove: { + * : // CSS declaration + * ... + * } + * } + * ... // more rules + * } + * } + * ... // more sources + */ +const INITIAL_STATE = {}; + const reducers = { - [TRACK_CHANGE](state, { data }) { + [TRACK_CHANGE](state, { change }) { const defaults = { - href: "", - selector: "", - tag: null, - add: null, - remove: null, + selector: null, + source: {}, + ancestors: [], + add: {}, + remove: {}, }; - data = { ...defaults, ...data }; + change = { ...defaults, ...change }; + state = cloneState(state); - // Update the state in-place with data about a style change (no deep clone of state). - // TODO: redefine state as a shallow object structure after figuring how to track - // both CSS Declarations and CSS Rules and At-Rules (@media, @keyframes, etc). - // @See https://bugzilla.mozilla.org/show_bug.cgi?id=1491263 - return Object.assign({}, { diff: updateDiff(state.diff, data) }); + const { type, href, index } = change.source; + const { selector, ancestors, ruleIndex } = change; + const sourceId = getSourceHash(change.source); + const ruleId = getRuleHash({ selector, ancestors, ruleIndex }); + + // Copy or create object identifying the source (styelsheet/element) for this change. + const source = Object.assign({}, state[sourceId], { type, href, index }); + // Copy or create collection of all rules ever changed in this source. + const rules = Object.assign({}, source.rules); + // Refrence or create object identifying the rule for this change. + let rule = rules[ruleId]; + if (!rule) { + rule = createRule({ selector, ancestors, ruleIndex }, rules); + } + // Copy or create collection of all CSS declarations ever added to this rule. + const add = Object.assign({}, rule.add); + // Copy or create collection of all CSS declarations ever removed from this rule. + const remove = Object.assign({}, rule.remove); + + if (change.remove && change.remove.property) { + // Track the remove operation only if the property was not previously introduced + // by an add operation. This ensures repeated changes of the same property + // register as a single remove operation of its original value. + if (!add[change.remove.property]) { + remove[change.remove.property] = change.remove.value; + } + + // Delete any previous add operation which would be canceled out by this remove. + if (add[change.remove.property] === change.remove.value) { + delete add[change.remove.property]; + } + } + + if (change.add && change.add.property) { + add[change.add.property] = change.add.value; + } + + const property = change.add && change.add.property || + change.remove && change.remove.property; + + // Remove tracked operations if they cancel each other out. + if (add[property] === remove[property]) { + delete add[property]; + delete remove[property]; + } + + // Remove information about the rule if none its declarations changed. + if (!Object.keys(add).length && !Object.keys(remove).length) { + removeRule(ruleId, rules); + source.rules = { ...rules }; + } else { + source.rules = { ...rules, [ruleId]: { ...rule, add, remove } }; + } + + // Remove information about the source if none of its rules changed. + if (!Object.keys(source.rules).length) { + delete state[sourceId]; + } else { + state[sourceId] = source; + } + + return state; }, [RESET_CHANGES](state) { diff --git a/devtools/client/inspector/changes/test/.eslintrc.js b/devtools/client/inspector/changes/test/.eslintrc.js new file mode 100644 index 000000000000..ed1e234b39bc --- /dev/null +++ b/devtools/client/inspector/changes/test/.eslintrc.js @@ -0,0 +1,6 @@ +"use strict"; + +module.exports = { + // Extend from the shared list of defined globals for mochitests. + "extends": "../../../../.eslintrc.mochitests.js", +}; diff --git a/devtools/client/inspector/changes/test/browser.ini b/devtools/client/inspector/changes/test/browser.ini new file mode 100644 index 000000000000..8e9b5dc5fd67 --- /dev/null +++ b/devtools/client/inspector/changes/test/browser.ini @@ -0,0 +1,17 @@ +[DEFAULT] +tags = devtools +subsuite = devtools +support-files = + head.js + !/devtools/client/inspector/test/head.js + !/devtools/client/inspector/test/shared-head.js + !/devtools/client/inspector/rules/test/head.js + !/devtools/client/shared/test/shared-head.js + !/devtools/client/shared/test/shared-redux-head.js + !/devtools/client/shared/test/telemetry-test-helpers.js + !/devtools/client/shared/test/test-actor.js + !/devtools/client/shared/test/test-actor-registry.js + +[browser_changes_declaration_disable.js] +[browser_changes_declaration_edit_value.js] +[browser_changes_declaration_remove.js] diff --git a/devtools/client/inspector/changes/test/browser_changes_declaration_disable.js b/devtools/client/inspector/changes/test/browser_changes_declaration_disable.js new file mode 100644 index 000000000000..34ea56854adc --- /dev/null +++ b/devtools/client/inspector/changes/test/browser_changes_declaration_disable.js @@ -0,0 +1,47 @@ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that toggling a CSS declaration in the Rule view is tracked. + +const TEST_URI = ` + +
+`; + +add_task(async function() { + await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + const { inspector, view: ruleView } = await openRuleView(); + const { document: doc, store } = selectChangesView(inspector); + const panel = doc.querySelector("#sidebar-panel-changes"); + + await selectNode("div", inspector); + const rule = getRuleViewRuleEditor(ruleView, 1).rule; + const prop = rule.textProps[0]; + + let onTrackChange = waitUntilAction(store, "TRACK_CHANGE"); + info("Disable the first declaration"); + await togglePropStatus(ruleView, prop); + info("Wait for change to be tracked"); + await onTrackChange; + + let removedDeclarations = panel.querySelectorAll(".diff-remove"); + is(removedDeclarations.length, 1, "Only one declaration was tracked as removed"); + + onTrackChange = waitUntilAction(store, "TRACK_CHANGE"); + info("Re-enable the first declaration"); + await togglePropStatus(ruleView, prop); + info("Wait for change to be tracked"); + await onTrackChange; + + const addedDeclarations = panel.querySelectorAll(".diff-add"); + removedDeclarations = panel.querySelectorAll(".diff-remove"); + is(addedDeclarations.length, 0, "No declarations were tracked as added"); + is(removedDeclarations.length, 0, "No declarations were tracked as removed"); +}); diff --git a/devtools/client/inspector/changes/test/browser_changes_declaration_edit_value.js b/devtools/client/inspector/changes/test/browser_changes_declaration_edit_value.js new file mode 100644 index 000000000000..13ebdf8e2e1b --- /dev/null +++ b/devtools/client/inspector/changes/test/browser_changes_declaration_edit_value.js @@ -0,0 +1,95 @@ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that editing the value of a CSS declaration in the Rule view is tracked. + +const TEST_URI = ` + +
+`; + +/* + This array will be iterated over in order and the `value` property will be used to + update the value of the first CSS declaration. + The `add` and `remove` objects hold the expected values of the tracked declarations + shown in the Changes panel. If `add` or `remove` are null, it means we don't expect + any corresponding tracked declaration to show up in the Changes panel. + */ +const VALUE_CHANGE_ITERATIONS = [ + // No changes should be tracked if the value did not actually change. + { + value: "red", + add: null, + remove: null, + }, + // Changing the priority flag "!important" should be tracked. + { + value: "red !important", + add: { value: "red !important" }, + remove: { value: "red" }, + }, + // Repeated changes should still show the original value as the one removed. + { + value: "blue", + add: { value: "blue" }, + remove: { value: "red" }, + }, + // Restoring the original value should clear tracked changes. + { + value: "red", + add: null, + remove: null, + }, +]; + +add_task(async function() { + await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + const { inspector, view: ruleView } = await openRuleView(); + const { document: doc, store } = selectChangesView(inspector); + const panel = doc.querySelector("#sidebar-panel-changes"); + + await selectNode("div", inspector); + const rule = getRuleViewRuleEditor(ruleView, 1).rule; + const prop = rule.textProps[0]; + + let onTrackChange; + let removeValue; + let addValue; + + for (const { value, add, remove } of VALUE_CHANGE_ITERATIONS) { + onTrackChange = waitUntilAction(store, "TRACK_CHANGE"); + + info(`Change the CSS declaration value to ${value}`); + await setProperty(ruleView, prop, value); + info("Wait for the change to be tracked"); + await onTrackChange; + + addValue = panel.querySelector(".diff-add .declaration-value"); + removeValue = panel.querySelector(".diff-remove .declaration-value"); + + if (add) { + is(addValue.textContent, add.value, + `Added declaration has expected value: ${add.value}`); + is(panel.querySelectorAll(".diff-add").length, 1, + "Only one declaration was tracked as added."); + } else { + ok(!addValue, `Added declaration was cleared`); + } + + if (remove) { + is(removeValue.textContent, remove.value, + `Removed declaration has expected value: ${remove.value}`); + is(panel.querySelectorAll(".diff-remove").length, 1, + "Only one declaration was tracked as removed."); + } else { + ok(!removeValue, `Removed declaration was cleared`); + } + } +}); diff --git a/devtools/client/inspector/changes/test/browser_changes_declaration_remove.js b/devtools/client/inspector/changes/test/browser_changes_declaration_remove.js new file mode 100644 index 000000000000..cd87ca852d36 --- /dev/null +++ b/devtools/client/inspector/changes/test/browser_changes_declaration_remove.js @@ -0,0 +1,38 @@ +/* vim: set ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that removing a CSS declaration from a rule in the Rule view is tracked. + +const TEST_URI = ` + +
+`; + +add_task(async function() { + await addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + const { inspector, view: ruleView } = await openRuleView(); + const { document: doc, store } = selectChangesView(inspector); + const panel = doc.querySelector("#sidebar-panel-changes"); + + await selectNode("div", inspector); + const rule = getRuleViewRuleEditor(ruleView, 1).rule; + const prop = rule.textProps[0]; + + const onTrackChange = waitUntilAction(store, "TRACK_CHANGE"); + info("Remove the first declaration"); + await removeProperty(ruleView, prop); + info("Wait for change to be tracked"); + await onTrackChange; + + const removeName = panel.querySelector(".diff-remove .declaration-name"); + const removeValue = panel.querySelector(".diff-remove .declaration-value"); + is(removeName.textContent, "color", "Correct declaration name was tracked as removed"); + is(removeValue.textContent, "red", "Correct declaration value was tracked as removed"); +}); diff --git a/devtools/client/inspector/changes/test/head.js b/devtools/client/inspector/changes/test/head.js new file mode 100644 index 000000000000..00b791f6d612 --- /dev/null +++ b/devtools/client/inspector/changes/test/head.js @@ -0,0 +1,31 @@ + /* vim: set ts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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/. */ +/* eslint no-unused-vars: [2, {"vars": "local"}] */ +/* import-globals-from ../../test/head.js */ +/* import-globals-from ../../../inspector/rules/test/head.js */ +/* import-globals-from ../../../inspector/test/shared-head.js */ +/* import-globals-from ../../../shared/test/shared-redux-head.js */ +"use strict"; + +// Load the Rule view's test/head.js to make use of its helpers. +// It loads inspector/test/head.js which itself loads inspector/test/shared-head.js +Services.scriptloader.loadSubScript( + "chrome://mochitests/content/browser/devtools/client/inspector/rules/test/head.js", + this); + +// Load the shared Redux helpers. +Services.scriptloader.loadSubScript( + "chrome://mochitests/content/browser/devtools/client/shared/test/shared-redux-head.js", + this); + +// Ensure the Changes panel is enabled before running the tests. +Services.prefs.setBoolPref("devtools.inspector.changes.enabled", true); +// Ensure the three-pane mode is enabled before running the tests. +Services.prefs.setBoolPref("devtools.inspector.three-pane-enabled", true); + +registerCleanupFunction(() => { + Services.prefs.clearUserPref("devtools.inspector.changes.enabled"); + Services.prefs.clearUserPref("devtools.inspector.three-pane-enabled"); +}); diff --git a/devtools/client/inspector/changes/utils/changes-utils.js b/devtools/client/inspector/changes/utils/changes-utils.js new file mode 100644 index 000000000000..a983345eb834 --- /dev/null +++ b/devtools/client/inspector/changes/utils/changes-utils.js @@ -0,0 +1,58 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public +* 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/. */ + +"use strict"; + +/** +* Generate a hash that uniquely identifies a stylesheet or element style attribute. +* +* @param {Object} source +* Information about a stylesheet or element style attribute: +* { +* type: {String} +* One of "stylesheet" or "element". +* index: {Number|String} +* Position of the styleshet in the list of stylesheets in the document. +* If `type` is "element", `index` is the generated selector which +* uniquely identifies the element in the document. +* href: {String|null} +* URL of the stylesheet or of the document when `type` is "element". +* If the stylesheet is inline, `href` is null. +* } +* @return {String} +*/ +function getSourceHash(source) { + const { type, index, href = "inline" } = source; + + return `${type}${index}${href}`; +} + +/** +* Generate a hash that uniquely identifies a CSS rule. +* +* @param {Object} ruleData +* Information about a CSS rule: +* { +* selector: {String} +* CSS selector text +* ancestors: {Array} +* Flattened CSS rule tree of the rule's ancestors with the root rule +* at the beginning of the array and the leaf rule at the end. +* ruleIndex: {Array} +* Indexes of each ancestor rule within its parent rule. +* } +* @return {String} +*/ +function getRuleHash(ruleData) { + const { selector = "", ancestors = [], ruleIndex } = ruleData; + const atRules = ancestors.reduce((acc, rule) => { + acc += `${rule.typeName} ${(rule.conditionText || rule.name || rule.keyText)}`; + return acc; + }, ""); + + return `${atRules}${selector}${ruleIndex}`; +} + +module.exports.getSourceHash = getSourceHash; +module.exports.getRuleHash = getRuleHash; diff --git a/devtools/client/inspector/changes/utils/moz.build b/devtools/client/inspector/changes/utils/moz.build new file mode 100644 index 000000000000..75be0cacd186 --- /dev/null +++ b/devtools/client/inspector/changes/utils/moz.build @@ -0,0 +1,9 @@ +# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- +# vim: set filetype=python: +# This Source Code Form is subject to the terms of the Mozilla Public +# 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/. + +DevToolsModules( + 'changes-utils.js', +) diff --git a/devtools/client/inspector/inspector.js b/devtools/client/inspector/inspector.js index a4aca0ce68b9..8e96fdb17360 100644 --- a/devtools/client/inspector/inspector.js +++ b/devtools/client/inspector/inspector.js @@ -278,7 +278,7 @@ Inspector.prototype = { // the ChangesActor. We want the ChangesActor to be guaranteed available before // the user makes any changes. this.changesFront = this.toolbox.target.getFront("changes"); - await this.changesFront.allChanges(); + this.changesFront.allChanges(); } // Setup the splitter before the sidebar is displayed so, we don't miss any events. diff --git a/devtools/client/inspector/test/shared-head.js b/devtools/client/inspector/test/shared-head.js index c69fcafd5b44..be43140e0763 100644 --- a/devtools/client/inspector/test/shared-head.js +++ b/devtools/client/inspector/test/shared-head.js @@ -120,6 +120,24 @@ function openComputedView() { }); } +/** + * Open the toolbox, with the inspector tool visible, and the changes view + * sidebar tab selected. + * + * @return a promise that resolves when the inspector is ready and the changes + * view is visible and ready + */ +function openChangesView() { + return openInspectorSidebarTab("changesview").then(data => { + return { + toolbox: data.toolbox, + inspector: data.inspector, + testActor: data.testActor, + view: data.inspector.changesView, + }; + }); +} + /** * Open the toolbox, with the inspector tool visible, and the layout view * sidebar tab selected to display the box model view with properties. @@ -176,6 +194,18 @@ function selectComputedView(inspector) { return inspector.getPanel("computedview").computedView; } +/** + * Select the changes view sidebar tab on an already opened inspector panel. + * + * @param {InspectorPanel} inspector + * The opened inspector panel + * @return {ChangesView} the changes view + */ +function selectChangesView(inspector) { + inspector.sidebar.select("changesview"); + return inspector.changesView; +} + /** * Select the layout view sidebar tab on an already opened inspector panel. * diff --git a/devtools/client/themes/changes.css b/devtools/client/themes/changes.css index 380b1d446779..20eb303236fc 100644 --- a/devtools/client/themes/changes.css +++ b/devtools/client/themes/changes.css @@ -8,70 +8,70 @@ --diff-add-text-color: #54983f; --diff-remove-background-color: #fbf2f5; --diff-remove-text-color: #bf7173; + --diff-level-offset: 15px; } #sidebar-panel-changes { margin: 0; + padding: 0; width: 100%; height: 100%; overflow: auto; } -details { - font-family: var(--monospace-font-family); - font-size: 12px; -} - -summary { - outline: none; - padding: 5px 0; - -moz-user-select: none; - cursor: pointer; -} - -details.source[open] { +#sidebar-panel-changes .source[open] { padding-bottom: 10px; } -details.source > summary { +#sidebar-panel-changes .href { background: var(--theme-sidebar-background); border-top: 1px solid var(--theme-splitter-color); border-bottom: 1px solid var(--theme-splitter-color); - padding-left: 5px; + padding: 5px; white-space: nowrap; text-overflow: ellipsis; overflow: hidden; + cursor: pointer; } -details.selector > summary { - padding-left: 10px; -} - -details.selector summary::after{ - content: "{…}"; - display: inline-block; - padding-left: 5px; -} - -details.selector[open]{ - margin-bottom: 5px; -} - -details.selector[open] summary::after{ - content: "{"; -} - -details.selector[open]::after{ - content: "}"; - display: block; - padding-left: 10px; -} - -.line { - padding: 3px 5px 3px 15px; +#sidebar-panel-changes .rule .level { + padding-top: 3px; + padding-right: 5px; + padding-bottom: 3px; + padding-left: var(--diff-level-offset); position: relative; } +#sidebar-panel-changes .rule > .rule .level { + padding-left: calc(var(--diff-level-offset) * 2); +} + +#sidebar-panel-changes .rule > .rule > .rule .level { + padding-left: calc(var(--diff-level-offset) * 3); +} + +#sidebar-panel-changes .rule > .rule > .rule > .rule .level { + padding-left: calc(var(--diff-level-offset) * 4); +} + +#sidebar-panel-changes .rule .selector, +#sidebar-panel-changes .rule .bracket-close { + margin-left: calc(-1 * var(--diff-level-offset) + 5px); +} + +#sidebar-panel-changes .rule .bracket-open { + display: inline-block; + margin-left: 5px; +} + +#sidebar-panel-changes .declaration-name { + margin-left: 10px; +} + +#sidebar-panel-changes .declaration-value { + margin-left: 5px; +} + .diff-add::before, .diff-remove::before{ position: absolute; diff --git a/devtools/server/actors/highlighters/flexbox.js b/devtools/server/actors/highlighters/flexbox.js index 099d0ea59309..42bff8e62c52 100644 --- a/devtools/server/actors/highlighters/flexbox.js +++ b/devtools/server/actors/highlighters/flexbox.js @@ -165,6 +165,9 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { this.clearCache(); this.flexData = null; + this.mainAxisDirection = null; + this.crossAxisDirection = null; + this.axes = null; AutoRefreshHighlighter.prototype.destroy.call(this); } @@ -301,8 +304,21 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { this.computedStyle = getComputedStyle(this.currentNode); } + const flex = this.currentNode.getAsFlexContainer(); + + const oldCrossAxisDirection = this.crossAxisDirection; + this.crossAxisDirection = flex.crossAxisDirection; + const newCrossAxisDirection = this.crossAxisDirection; + + const oldMainAxisDirection = this.mainAxisDirection; + this.mainAxisDirection = flex.mainAxisDirection; + const newMainAxisDirection = this.mainAxisDirection; + + // Concatenate the axes to simplify conditionals. + this.axes = `${this.mainAxisDirection} ${this.crossAxisDirection}`; + const oldFlexData = this.flexData; - this.flexData = getFlexData(this.currentNode.getAsFlexContainer(), this.win); + this.flexData = getFlexData(flex, this.win); const hasFlexDataChanged = compareFlexData(oldFlexData, this.flexData); const oldAlignItems = this.alignItemsValue; @@ -326,7 +342,9 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { oldAlignItems !== newAlignItems || oldFlexDirection !== newFlexDirection || oldFlexWrap !== newFlexWrap || - oldJustifyContent !== newJustifyContent; + oldJustifyContent !== newJustifyContent || + oldCrossAxisDirection !== newCrossAxisDirection || + oldMainAxisDirection !== newMainAxisDirection; } _hide() { @@ -588,50 +606,78 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { this.ctx.strokeStyle = this.color; const { bounds } = this.currentQuads.content[0]; - const isColumn = this.flexDirection.startsWith("column"); const options = { matrix: this.currentMatrix }; for (const flexLine of this.flexData.lines) { const { crossStart, crossSize } = flexLine; - if (isColumn) { - clearRect(this.ctx, crossStart, 0, crossStart + crossSize, bounds.height, - this.currentMatrix); + switch (this.axes) { + case "horizontal-lr vertical-tb": + case "horizontal-lr vertical-bt": + case "horizontal-rl vertical-tb": + case "horizontal-rl vertical-bt": + clearRect(this.ctx, 0, crossStart, bounds.width, crossStart + crossSize, + this.currentMatrix); - // Avoid drawing the start flex line when they overlap with the flex container. - if (crossStart != 0) { - drawLine(this.ctx, crossStart, 0, crossStart, bounds.height, options); - this.ctx.stroke(); - } + // Avoid drawing the start flex line when they overlap with the flex container. + if (crossStart != 0) { + drawLine(this.ctx, 0, crossStart, bounds.width, crossStart, options); + this.ctx.stroke(); + } - // Avoid drawing the end flex line when they overlap with the flex container. - if (bounds.width - crossStart - crossSize >= lineWidth) { - drawLine(this.ctx, crossStart + crossSize, 0, crossStart + crossSize, - bounds.height, options); - this.ctx.stroke(); - } - } else { - clearRect(this.ctx, 0, crossStart, bounds.width, crossStart + crossSize, - this.currentMatrix); + // Avoid drawing the end flex line when they overlap with the flex container. + if (bounds.height - crossStart - crossSize >= lineWidth) { + drawLine(this.ctx, 0, crossStart + crossSize, bounds.width, + crossStart + crossSize, options); + this.ctx.stroke(); + } + break; + case "vertical-tb horizontal-lr": + case "vertical-bt horizontal-rl": + clearRect(this.ctx, crossStart, 0, crossStart + crossSize, bounds.height, + this.currentMatrix); - // Avoid drawing the start flex line when they overlap with the flex container. - if (crossStart != 0) { - drawLine(this.ctx, 0, crossStart, bounds.width, crossStart, options); - this.ctx.stroke(); - } + // Avoid drawing the start flex line when they overlap with the flex container. + if (crossStart != 0) { + drawLine(this.ctx, crossStart, 0, crossStart, bounds.height, options); + this.ctx.stroke(); + } - // Avoid drawing the end flex line when they overlap with the flex container. - if (bounds.height - crossStart - crossSize >= lineWidth) { - drawLine(this.ctx, 0, crossStart + crossSize, bounds.width, - crossStart + crossSize, options); - this.ctx.stroke(); - } + // Avoid drawing the end flex line when they overlap with the flex container. + if (bounds.width - crossStart - crossSize >= lineWidth) { + drawLine(this.ctx, crossStart + crossSize, 0, crossStart + crossSize, + bounds.height, options); + this.ctx.stroke(); + } + break; + case "vertical-bt horizontal-lr": + case "vertical-tb horizontal-rl": + clearRect(this.ctx, bounds.width - crossStart, 0, + bounds.width - crossStart - crossSize, bounds.height, this.currentMatrix); + + // Avoid drawing the start flex line when they overlap with the flex container. + if (crossStart != 0) { + drawLine(this.ctx, bounds.width - crossStart, 0, bounds.width - crossStart, + bounds.height, options); + this.ctx.stroke(); + } + + // Avoid drawing the end flex line when they overlap with the flex container. + if (bounds.width - crossStart - crossSize >= lineWidth) { + drawLine(this.ctx, bounds.width - crossStart - crossSize, 0, + bounds.width - crossStart - crossSize, bounds.height, options); + this.ctx.stroke(); + } + break; } } this.ctx.restore(); } + /** + * Clear the whole alignment container along the main axis for each flex item. + */ renderJustifyContent() { if (!this.flexData || !this.currentQuads.content || !this.currentQuads.content[0]) { return; @@ -639,11 +685,12 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { const zoom = getCurrentZoom(this.win); const { bounds } = this.currentQuads.content[0]; - const isColumn = this.flexDirection.startsWith("column"); + + // Draw a justify content pattern over the whole flex container. + this.drawJustifyContent(0, 0, bounds.width, bounds.height, this.currentMatrix); for (const flexLine of this.flexData.lines) { const { crossStart, crossSize } = flexLine; - let mainStart = 0; for (const flexItem of flexLine.items) { const quads = flexItem.quads; @@ -658,23 +705,27 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { const right = Math.round(flexItemBounds.right / zoom - bounds.left); const bottom = Math.round(flexItemBounds.bottom / zoom - bounds.top); - if (isColumn) { - this.drawJustifyContent(crossStart, mainStart, crossStart + crossSize, top); - mainStart = bottom; - } else { - this.drawJustifyContent(mainStart, crossStart, left, crossStart + crossSize); - mainStart = right; + // Clear a rectangular are covering the alignment container. + switch (this.axes) { + case "horizontal-lr vertical-tb": + case "horizontal-lr vertical-bt": + case "horizontal-rl vertical-tb": + case "horizontal-rl vertical-bt": + clearRect(this.ctx, left, Math.round(crossStart) + 2, right, + Math.round(crossStart + crossSize) - 2, this.currentMatrix); + break; + case "vertical-tb horizontal-lr": + case "vertical-bt horizontal-rl": + clearRect(this.ctx, Math.round(crossStart) + 1, top, + Math.round(crossStart + crossSize), bottom, this.currentMatrix); + break; + case "vertical-bt horizontal-lr": + case "vertical-tb horizontal-rl": + clearRect(this.ctx, Math.round(bounds.width - crossStart - crossSize) + 1, + top, Math.round(bounds.width - crossStart), bottom, this.currentMatrix); + break; } } - - // Draw the last justify-content area after the last flex item. - if (isColumn) { - this.drawJustifyContent(crossStart, mainStart, crossStart + crossSize, - bounds.height); - } else { - this.drawJustifyContent(mainStart, crossStart, bounds.width, - crossStart + crossSize); - } } } diff --git a/devtools/server/actors/styles.js b/devtools/server/actors/styles.js index d89defce90cc..1ddad84e2d6c 100644 --- a/devtools/server/actors/styles.js +++ b/devtools/server/actors/styles.js @@ -27,7 +27,10 @@ loader.lazyRequireGetter(this, "UPDATE_PRESERVING_RULES", "devtools/server/actors/stylesheets", true); loader.lazyRequireGetter(this, "UPDATE_GENERAL", "devtools/server/actors/stylesheets", true); -loader.lazyRequireGetter(this, "findCssSelector", "devtools/shared/inspector/css-logic", true); +loader.lazyRequireGetter(this, "findCssSelector", + "devtools/shared/inspector/css-logic", true); +loader.lazyRequireGetter(this, "CSSRuleTypeName", + "devtools/shared/inspector/css-logic", true); loader.lazyGetter(this, "PSEUDO_ELEMENTS", () => { return InspectorUtils.getCSSPseudoElementNames(); @@ -986,13 +989,13 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { if (CSSRule.isInstance(item)) { this.type = item.type; this.rawRule = item; + this._computeRuleIndex(); if ((this.type === CSSRule.STYLE_RULE || this.type === CSSRule.KEYFRAME_RULE) && this.rawRule.parentStyleSheet) { this.line = InspectorUtils.getRelativeRuleLine(this.rawRule); this.column = InspectorUtils.getRuleColumn(this.rawRule); this._parentSheet = this.rawRule.parentStyleSheet; - this._computeRuleIndex(); this.sheetActor = this.pageStyle._sheetRef(this._parentSheet); this.sheetActor.on("style-applied", this._onStyleApplied); } @@ -1049,6 +1052,25 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { this._parentSheet.href !== "about:PreferenceStyleSheet"); }, + /** + * Return an array with StyleRuleActor instances for each of this rule's ancestor rules + * (@media, @supports, @keyframes, etc) obtained by recursively reading rule.parentRule. + * If the rule has no ancestors, return an empty array. + * + * @return {Array} + */ + get ancestorRules() { + const ancestors = []; + let rule = this.rawRule; + + while (rule.parentRule) { + ancestors.push(this.pageStyle._styleRef(rule.parentRule)); + rule = rule.parentRule; + } + + return ancestors; + }, + getDocument: function(sheet) { if (sheet.ownerNode) { return sheet.ownerNode.nodeType == sheet.ownerNode.DOCUMENT_NODE ? @@ -1186,10 +1208,10 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { const result = []; while (rule) { - let cssRules; + let cssRules = []; if (rule.parentRule) { cssRules = rule.parentRule.cssRules; - } else { + } else if (rule.parentStyleSheet) { cssRules = rule.parentStyleSheet.cssRules; } @@ -1467,28 +1489,69 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { * Data about a modification to a rule. @see |modifyProperties()| */ logChange(change) { - const prevValue = this._declarations[change.index] - ? this._declarations[change.index].value - : null; - const prevName = this._declarations[change.index] - ? this._declarations[change.index].name - : null; + // Destructure properties from the previous CSS declaration at this index, if any, + // to new variable names to indicate the previous state. + let { + value: prevValue, + name: prevName, + priority: prevPriority, + commentOffsets, + } = this._declarations[change.index] || {}; + // A declaration is disabled if it has a `commentOffsets` array. + // Here we type coerce the value to a boolean with double-bang (!!) + const prevDisabled = !!commentOffsets; + // Append the "!important" string if defined in the previous priority flag. + prevValue = (prevValue && prevPriority) ? `${prevValue} !important` : prevValue; // Metadata about a change. const data = {}; - data.type = change.type; - data.selector = this.rawRule.selectorText; + // Collect information about the rule's ancestors (@media, @supports, @keyframes). + // Used to show context for this change in the UI and to match the rule for undo/redo. + data.ancestors = this.ancestorRules.map(rule => { + return { + // Rule type as number defined by CSSRule.type (ex: 4, 7, 12) + // @see https://developer.mozilla.org/en-US/docs/Web/API/CSSRule + type: rule.rawRule.type, + // Rule type as human-readable string (ex: "@media", "@supports", "@keyframes") + typeName: CSSRuleTypeName[rule.rawRule.type], + // Conditions of @media and @supports rules (ex: "min-width: 1em") + conditionText: rule.rawRule.conditionText, + // Name of @keyframes rule; refrenced by the animation-name CSS property. + name: rule.rawRule.name, + // Selector of individual @keyframe rule within a @keyframes rule (ex: 0%, 100%). + keyText: rule.rawRule.keyText, + // Array with the indexes of this rule and its ancestors within the CSS rule tree. + ruleIndex: rule._ruleIndex, + }; + }); - // For inline style changes, generate a unique selector and pass the node tag. + // For changes in element style attributes, generate a unique selector. if (this.type === ELEMENT_STYLE) { - data.tag = this.rawNode.tagName; - data.href = "inline"; // findCssSelector() fails on XUL documents. Catch and silently ignore that error. try { data.selector = findCssSelector(this.rawNode); } catch (err) {} + + data.source = { + type: "element", + // Used to differentiate between elements which match the same generated selector + // but live in different documents (ex: host document and iframe). + href: this.rawNode.baseURI, + // Element style attributes don't have a rule index; use the generated selector. + index: data.selector, + }; + data.ruleIndex = 0; } else { - data.href = this._parentSheet.href || "inline stylesheet"; + data.selector = (this.type === CSSRule.KEYFRAME_RULE) + ? this.rawRule.keyText + : this.rawRule.selectorText; + data.source = { + type: "stylesheet", + href: this.sheetActor.href, + index: this.sheetActor.styleSheetIndex, + }; + // Used to differentiate between changes to rules with identical selectors. + data.ruleIndex = this._ruleIndex; } switch (change.type) { @@ -1497,14 +1560,26 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { // Otherwise, a new declaration is being created or the value of an existing // declaration is being updated. In that case, use the provided `change.name`. const name = change.newName ? change.newName : change.name; - // Reuse the previous value when the property is being renamed. - const value = change.newName ? prevValue : change.value; + // Append the "!important" string if defined in the incoming priority flag. + const newValue = change.priority ? `${change.value} !important` : change.value; + // Reuse the previous value string, when the property is renamed. + // Otherwise, use the incoming value string. + const value = change.newName ? prevValue : newValue; data.add = { property: name, value }; // If there is a previous value, log its removal together with the previous // property name. Using the previous name handles the case for renaming a property // and is harmless when updating an existing value (the name stays the same). data.remove = prevValue ? { property: prevName, value: prevValue } : null; + + // When toggling a declaration from OFF to ON, if not renaming the property, + // do not mark the previous declaration for removal, otherwise the add and + // remove operations will cancel each other out when tracked. Tracked changes + // have no context of "disabled", only "add" or remove, like diffs. + if (prevDisabled && !change.newName && prevValue === newValue) { + data.remove = null; + } + break; case "remove": @@ -1513,14 +1588,6 @@ var StyleRuleActor = protocol.ActorClassWithSpec(styleRuleSpec, { break; } - // Do not track non-changes. This can occur when typing a value in the Rule view - // inline editor, then committing it by pressing the Enter key. - if (data.add && data.remove && - data.add.property === data.remove.property && - data.add.value === data.remove.value) { - return; - } - TrackChangeEmitter.trackChange(data); }, diff --git a/devtools/shared/css/parsing-utils.js b/devtools/shared/css/parsing-utils.js index 4d174e64178f..47e5a93e86e6 100644 --- a/devtools/shared/css/parsing-utils.js +++ b/devtools/shared/css/parsing-utils.js @@ -875,6 +875,7 @@ RuleRewriter.prototype = { setPropertyEnabled: function(index, name, isEnabled) { this.completeInitialization(index); const decl = this.decl; + const priority = decl.priority; let copyOffset = decl.offsets[1]; if (isEnabled) { // Enable it. First see if the comment start can be deleted. @@ -918,9 +919,9 @@ RuleRewriter.prototype = { this.completeCopying(copyOffset); if (isEnabled) { - this.modifications.push({ type: "set", index, name, value: decl.value }); + this.modifications.push({ type: "set", index, name, value: decl.value, priority }); } else { - this.modifications.push({ type: "remove", index, name }); + this.modifications.push({ type: "remove", index, name, priority }); } }, diff --git a/devtools/shared/inspector/css-logic.js b/devtools/shared/inspector/css-logic.js index 98e42b349a16..f81995b48316 100644 --- a/devtools/shared/inspector/css-logic.js +++ b/devtools/shared/inspector/css-logic.js @@ -82,6 +82,26 @@ exports.STATUS = { UNKNOWN: -1, }; +/** + * Mapping of CSSRule type value to CSSRule type name. + * @see https://developer.mozilla.org/en-US/docs/Web/API/CSSRule + */ +exports.CSSRuleTypeName = { + 1: "", // Regular CSS style rule has no name + 3: "@import", + 4: "@media", + 5: "@font-face", + 6: "@page", + 7: "@keyframes", + 8: "@keyframe", + 10: "@namespace", + 11: "@counter-style", + 12: "@supports", + 13: "@document", + 14: "@font-feature-values", + 15: "@viewport", +}; + /** * Lookup a l10n string in the shared styleinspector string bundle. * diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index 1f11a7930153..f65328b7d39a 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -5784,6 +5784,31 @@ nsGlobalWindowOuter::PostMessageMozOuter(JSContext* aCx, JS::Handle a if (NS_WARN_IF(!providedPrincipal)) { return; } + } else { + // We still need to check the originAttributes if the target origin is '*'. + // But we will ingore the FPD here since the FPDs are possible to be different. + auto principal = BasePrincipal::Cast(GetPrincipal()); + NS_ENSURE_TRUE_VOID(principal); + + OriginAttributes targetAttrs = principal->OriginAttributesRef(); + OriginAttributes sourceAttrs = aSubjectPrincipal.OriginAttributesRef(); + // We have to exempt the check of OA if the subject prioncipal is a system + // principal since there are many tests try to post messages to content from + // chrome with a mismatch OA. For example, using the ContentTask.spawn() to + // post a message into a private browsing window. The injected code in + // ContentTask.spawn() will be executed under the system principal and the + // OA of the system principal mismatches with the OA of a private browsing + // window. + MOZ_DIAGNOSTIC_ASSERT(aSubjectPrincipal.GetIsSystemPrincipal() || + sourceAttrs.EqualsIgnoringFPD(targetAttrs)); + + // If 'privacy.firstparty.isolate.block_post_message' is true, we will block + // postMessage across different first party domains. + if (OriginAttributes::IsBlockPostMessageForFPI() && + !aSubjectPrincipal.GetIsSystemPrincipal() && + sourceAttrs.mFirstPartyDomain != targetAttrs.mFirstPartyDomain) { + return; + } } // Create and asynchronously dispatch a runnable which will handle actual DOM diff --git a/gfx/webrender/src/batch.rs b/gfx/webrender/src/batch.rs index 21d9d5e86f8d..296f78a2b964 100644 --- a/gfx/webrender/src/batch.rs +++ b/gfx/webrender/src/batch.rs @@ -1294,7 +1294,8 @@ impl AlphaBatchBuilder { for (segment_index, (segment, segment_data)) in segment_desc.segments .iter() .zip(segment_data.iter()) - .enumerate() { + .enumerate() + { self.add_segment_to_batch( segment, segment_data, @@ -1315,7 +1316,8 @@ impl AlphaBatchBuilder { // between all segments. for (segment_index, segment) in segment_desc.segments .iter() - .enumerate() { + .enumerate() + { self.add_segment_to_batch( segment, segment_data, @@ -1333,6 +1335,7 @@ impl AlphaBatchBuilder { } (None, SegmentDataKind::Shared(ref segment_data)) => { // No segments, and thus no per-segment instance data. + // Note: the blend mode already takes opacity into account let batch_key = BatchKey { blend_mode: non_segmented_blend_mode, kind: BatchKind::Brush(params.batch_kind), diff --git a/gfx/webrender/src/prim_store.rs b/gfx/webrender/src/prim_store.rs index 2b6246871050..1b452a876ece 100644 --- a/gfx/webrender/src/prim_store.rs +++ b/gfx/webrender/src/prim_store.rs @@ -1932,7 +1932,8 @@ impl PrimitiveStore { prim_instance.clipped_world_rect = Some(pic_state.map_pic_to_world.bounds); } else { if prim.local_rect.size.width <= 0.0 || - prim.local_rect.size.height <= 0.0 { + prim.local_rect.size.height <= 0.0 + { if cfg!(debug_assertions) && is_chased { println!("\tculled for zero local rectangle"); } @@ -1979,6 +1980,9 @@ impl PrimitiveStore { let clip_chain = match clip_chain { Some(clip_chain) => clip_chain, None => { + if cfg!(debug_assertions) && is_chased { + println!("\tunable to build the clip chain, skipping"); + } prim_instance.clipped_world_rect = None; return false; } @@ -2085,8 +2089,8 @@ impl PrimitiveStore { let prim_index = prim_instance.prim_index; let is_chased = Some(prim_index) == self.chase_id; - if is_chased { - println!("\tpreparing prim {:?} in pipeline {:?}", + if cfg!(debug_assertions) && is_chased { + println!("\tpreparing {:?} in {:?}", prim_instance.prim_index, pic_context.pipeline_id); } @@ -2579,7 +2583,7 @@ impl PrimitiveInstance { self.prepared_frame_id = frame_state.render_tasks.frame_id(); } - match *prim_details { + self.opacity = match *prim_details { PrimitiveDetails::TextRun(ref mut text) => { // The transform only makes sense for screen space rasterization let transform = prim_context.spatial_node.world_content_transform.to_transform(); @@ -2591,6 +2595,7 @@ impl PrimitiveInstance { display_list, frame_state, ); + PrimitiveOpacity::translucent() } PrimitiveDetails::Brush(ref mut brush) => { match brush.kind { @@ -2620,13 +2625,6 @@ impl PrimitiveInstance { frame_state.gpu_cache.invalidate(&mut self.gpu_location); } - // Update opacity for this primitive to ensure the correct - // batching parameters are used. - self.opacity.is_opaque = - image_properties.descriptor.is_opaque && - opacity_binding.current == 1.0 && - color.a == 1.0; - if *tile_spacing != LayoutSize::zero() && !is_tiled { *source = ImageSource::Cache { // Size in device-pixels we need to allocate in render task cache. @@ -2648,6 +2646,7 @@ impl PrimitiveInstance { } let mut request_source_image = false; + let mut is_opaque = image_properties.descriptor.is_opaque; // Every frame, for cached items, we need to request the render // task cache item. The closure will be invoked on the first @@ -2666,9 +2665,7 @@ impl PrimitiveInstance { size.width += padding.horizontal(); size.height += padding.vertical(); - if padding != DeviceIntSideOffsets::zero() { - self.opacity.is_opaque = false; - } + is_opaque &= padding == DeviceIntSideOffsets::zero(); let image_cache_key = ImageCacheKey { request, @@ -2807,11 +2804,18 @@ impl PrimitiveInstance { frame_state.gpu_cache, ); } + + if is_opaque { + PrimitiveOpacity::from_alpha(opacity_binding.current * color.a) + } else { + PrimitiveOpacity::translucent() + } + } else { + PrimitiveOpacity::opaque() } } - BrushKind::LineDecoration { ref mut handle, style, orientation, wavy_line_thickness, .. } => { + BrushKind::LineDecoration { color, ref mut handle, style, orientation, wavy_line_thickness } => { // Work out the device pixel size to be used to cache this line decoration. - let size = get_line_decoration_sizes( &prim_local_rect.size, orientation, @@ -2819,6 +2823,10 @@ impl PrimitiveInstance { wavy_line_thickness, ); + if cfg!(debug_assertions) && is_chased { + println!("\tline decoration opaque={}, sizes={:?}", self.opacity.is_opaque, size); + } + if let Some((inline_size, block_size)) = size { let size = match orientation { LineOrientation::Horizontal => LayoutSize::new(inline_size, block_size), @@ -2887,10 +2895,15 @@ impl PrimitiveInstance { } )); } + + match style { + LineStyle::Solid => PrimitiveOpacity::from_alpha(color.a), + LineStyle::Dotted | + LineStyle::Dashed | + LineStyle::Wavy => PrimitiveOpacity::translucent(), + } } BrushKind::YuvImage { format, yuv_key, image_rendering, .. } => { - self.opacity = PrimitiveOpacity::opaque(); - let channel_num = format.get_plane_num(); debug_assert!(channel_num <= 3); for channel in 0 .. channel_num { @@ -2903,6 +2916,8 @@ impl PrimitiveInstance { frame_state.gpu_cache, ); } + + PrimitiveOpacity::opaque() } BrushKind::Border { ref mut source, .. } => { match *source { @@ -2912,15 +2927,15 @@ impl PrimitiveInstance { .get_image_properties(request.key); if let Some(image_properties) = image_properties { - // Update opacity for this primitive to ensure the correct - // batching parameters are used. - self.opacity.is_opaque = - image_properties.descriptor.is_opaque; - frame_state.resource_cache.request_image( request, frame_state.gpu_cache, ); + PrimitiveOpacity { + is_opaque: image_properties.descriptor.is_opaque, + } + } else { + PrimitiveOpacity::opaque() } } BorderSource::Border { ref border, ref widths, ref mut segments, .. } => { @@ -2963,6 +2978,9 @@ impl PrimitiveInstance { } )); } + + // Shouldn't matter, since the segment opacity is used instead + PrimitiveOpacity::translucent() } } } @@ -3015,6 +3033,9 @@ impl PrimitiveInstance { }, ); } + + //TODO: can we make it opaque in some cases? + PrimitiveOpacity::translucent() } BrushKind::LinearGradient { stops_range, @@ -3029,19 +3050,6 @@ impl PrimitiveInstance { ref mut visible_tiles, .. } => { - // If the coverage of the gradient extends to or beyond - // the primitive rect, then the opacity can be determined - // by the colors of the stops. If we have tiling / spacing - // then we just assume the gradient is translucent for now. - // (In the future we could consider segmenting in some cases). - let stride = stretch_size + tile_spacing; - self.opacity = if stride.width >= prim_local_rect.size.width && - stride.height >= prim_local_rect.size.height { - stops_opacity - } else { - PrimitiveOpacity::translucent() - }; - build_gradient_stops_request( stops_handle, stops_range, @@ -3078,6 +3086,19 @@ impl PrimitiveInstance { } ); } + + // If the coverage of the gradient extends to or beyond + // the primitive rect, then the opacity can be determined + // by the colors of the stops. If we have tiling / spacing + // then we just assume the gradient is translucent for now. + // (In the future we could consider segmenting in some cases). + let stride = stretch_size + tile_spacing; + if stride.width >= prim_local_rect.size.width && + stride.height >= prim_local_rect.size.height { + stops_opacity + } else { + PrimitiveOpacity::translucent() + } } BrushKind::Picture { pic_index, .. } => { let pic = &mut pictures[pic_index.0]; @@ -3101,6 +3122,8 @@ impl PrimitiveInstance { } else { self.clipped_world_rect = None; } + + PrimitiveOpacity::translucent() } BrushKind::Solid { ref color, ref mut opacity_binding, .. } => { // If the opacity changed, invalidate the GPU cache so that @@ -3110,12 +3133,12 @@ impl PrimitiveInstance { if opacity_binding.update(frame_context.scene_properties) { frame_state.gpu_cache.invalidate(&mut self.gpu_location); } - self.opacity = PrimitiveOpacity::from_alpha(opacity_binding.current * color.a); + PrimitiveOpacity::from_alpha(opacity_binding.current * color.a) } - BrushKind::Clear => {} + BrushKind::Clear => PrimitiveOpacity::opaque(), } } - } + }; if is_tiled { // we already requested each tile's gpu data. diff --git a/gfx/webrender/src/render_task.rs b/gfx/webrender/src/render_task.rs index f514c4dc611c..6903580cc879 100644 --- a/gfx/webrender/src/render_task.rs +++ b/gfx/webrender/src/render_task.rs @@ -52,7 +52,7 @@ fn render_task_sanity_check(size: &DeviceIntSize) { #[cfg_attr(feature = "replay", derive(Deserialize))] pub struct RenderTaskId(pub u32, FrameId); // TODO(gw): Make private when using GPU cache! -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, PartialEq)] #[repr(C)] #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] diff --git a/gfx/webrender_bindings/revision.txt b/gfx/webrender_bindings/revision.txt index 60057d4c9b92..36681addf749 100644 --- a/gfx/webrender_bindings/revision.txt +++ b/gfx/webrender_bindings/revision.txt @@ -1 +1 @@ -925dd051a36f1c1cd02e7f635f7e439ab2804f15 +2537e5f27c2ce7b64a93498c7569a870c190feda diff --git a/mobile/android/config/mozconfigs/android-aarch64/nightly b/mobile/android/config/mozconfigs/android-aarch64/nightly index aa0bf9798a5c..963ab380eaac 100644 --- a/mobile/android/config/mozconfigs/android-aarch64/nightly +++ b/mobile/android/config/mozconfigs/android-aarch64/nightly @@ -6,15 +6,6 @@ ac_add_options --target=aarch64-linux-android ac_add_options --with-branding=mobile/android/branding/nightly -export AR="$topsrcdir/clang/bin/llvm-ar" -export NM="$topsrcdir/clang/bin/llvm-nm" -export RANLIB="$topsrcdir/clang/bin/llvm-ranlib" - -# Enable LTO if the NDK is available. -if [ -z "$NO_NDK" ]; then - ac_add_options --enable-lto -fi - export MOZILLA_OFFICIAL=1 export MOZ_TELEMETRY_REPORTING=1 export MOZ_ANDROID_POCKET=1 diff --git a/mobile/android/config/mozconfigs/android-api-16/nightly b/mobile/android/config/mozconfigs/android-api-16/nightly index 098ba91b6254..ed0a3db43eef 100644 --- a/mobile/android/config/mozconfigs/android-api-16/nightly +++ b/mobile/android/config/mozconfigs/android-api-16/nightly @@ -16,13 +16,4 @@ export MOZ_TELEMETRY_REPORTING=1 export MOZ_ANDROID_MMA=1 export MOZ_ANDROID_POCKET=1 -export AR="$topsrcdir/clang/bin/llvm-ar" -export NM="$topsrcdir/clang/bin/llvm-nm" -export RANLIB="$topsrcdir/clang/bin/llvm-ranlib" - -# Enable LTO if the NDK is available. -if [ -z "$NO_NDK" ]; then - ac_add_options --enable-lto -fi - . "$topsrcdir/mobile/android/config/mozconfigs/common.override" diff --git a/mobile/android/config/mozconfigs/android-x86/nightly b/mobile/android/config/mozconfigs/android-x86/nightly index 8c3a10b0c1c7..a5346d28ce15 100644 --- a/mobile/android/config/mozconfigs/android-x86/nightly +++ b/mobile/android/config/mozconfigs/android-x86/nightly @@ -14,13 +14,4 @@ export MOZILLA_OFFICIAL=1 export MOZ_TELEMETRY_REPORTING=1 export MOZ_ANDROID_POCKET=1 -export AR="$topsrcdir/clang/bin/llvm-ar" -export NM="$topsrcdir/clang/bin/llvm-nm" -export RANLIB="$topsrcdir/clang/bin/llvm-ranlib" - -# Enable LTO if the NDK is available. -if [ -z "$NO_NDK" ]; then - ac_add_options --enable-lto -fi - . "$topsrcdir/mobile/android/config/mozconfigs/common.override" diff --git a/mobile/android/modules/DownloadNotifications.jsm b/mobile/android/modules/DownloadNotifications.jsm index a2f30c7843bc..54d69fc330e0 100644 --- a/mobile/android/modules/DownloadNotifications.jsm +++ b/mobile/android/modules/DownloadNotifications.jsm @@ -180,8 +180,10 @@ var DownloadNotifications = { }; function getCookieFromDownload(download) { + // Arbitrary value used to truncate long Data URLs. See bug 1497526 + const maxUrlLength = 1024; return download.target.path + - download.source.url + + download.source.url.slice(-maxUrlLength) + download.startTime; } diff --git a/netwerk/base/nsIRequest.idl b/netwerk/base/nsIRequest.idl index 52ee5bff2635..6f100e67cc59 100644 --- a/netwerk/base/nsIRequest.idl +++ b/netwerk/base/nsIRequest.idl @@ -224,6 +224,8 @@ interface nsIRequest : nsISupports /** * When set, this flag indicates that caches of network connections, * particularly HTTP persistent connections, should not be used. + * Use this together with LOAD_INITIAL_DOCUMENT_URI as otherwise it has no + * effect. */ const unsigned long LOAD_FRESH_CONNECTION = 1 << 15; }; diff --git a/taskcluster/docker/funsize-update-generator/runme.sh b/taskcluster/docker/funsize-update-generator/runme.sh index 111266a2ef81..3f76b8222850 100644 --- a/taskcluster/docker/funsize-update-generator/runme.sh +++ b/taskcluster/docker/funsize-update-generator/runme.sh @@ -43,8 +43,8 @@ then fi set -x else - # enable locale cache - export MBSDIFF_HOOK="/home/worker/bin/mbsdiff_hook.sh -c /tmp/fs-cache" + # disable caching + export MBSDIFF_HOOK= fi if [ ! -z "$FILENAME_TEMPLATE" ]; then diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index 197271d70a48..adeae0401991 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -1105,8 +1105,8 @@ var PlacesUtils = { */ validatePageInfo(pageInfo, validateVisits = true) { return this.validateItemProperties("PageInfo", PAGEINFO_VALIDATORS, pageInfo, - { url: { requiredIf: b => { typeof b.guid != "string"; } }, - guid: { requiredIf: b => { typeof b.url != "string"; } }, + { url: { requiredIf: b => !b.guid }, + guid: { requiredIf: b => !b.url }, visits: { requiredIf: b => validateVisits }, }); }, diff --git a/toolkit/components/places/SQLFunctions.cpp b/toolkit/components/places/SQLFunctions.cpp index a68ea1d2fe1c..f00ae4174bd9 100644 --- a/toolkit/components/places/SQLFunctions.cpp +++ b/toolkit/components/places/SQLFunctions.cpp @@ -446,55 +446,6 @@ namespace places { return findInString(aToken, aSourceString, eFindOnBoundary); } - /* static */ - bool - MatchAutoCompleteFunction::findBeginning(const nsDependentCSubstring &aToken, - const nsACString &aSourceString) - { - MOZ_ASSERT(!aToken.IsEmpty(), "Don't search for an empty token!"); - - // We can't use StringBeginsWith here, unfortunately. Although it will - // happily take a case-insensitive UTF8 comparator, it eventually calls - // nsACString::Equals, which checks that the two strings contain the same - // number of bytes before calling the comparator. Two characters may be - // case-insensitively equal while taking up different numbers of bytes, so - // this is not what we want. - - const_char_iterator tokenStart(aToken.BeginReading()), - tokenEnd(aToken.EndReading()), - sourceStart(aSourceString.BeginReading()), - sourceEnd(aSourceString.EndReading()); - - bool dummy; - while (sourceStart < sourceEnd && - CaseInsensitiveUTF8CharsEqual(sourceStart, tokenStart, - sourceEnd, tokenEnd, - &sourceStart, &tokenStart, &dummy)) { - - // We found the token! - if (tokenStart >= tokenEnd) { - return true; - } - } - - // We don't need to check CaseInsensitiveUTF8CharsEqual's error condition - // (stored in |dummy|), since the function will return false if it - // encounters an error. - - return false; - } - - /* static */ - bool - MatchAutoCompleteFunction::findBeginningCaseSensitive( - const nsDependentCSubstring &aToken, - const nsACString &aSourceString) - { - MOZ_ASSERT(!aToken.IsEmpty(), "Don't search for an empty token!"); - - return StringBeginsWith(aSourceString, aToken); - } - /* static */ MatchAutoCompleteFunction::searchFunctionPtr MatchAutoCompleteFunction::getSearchFunction(int32_t aBehavior) @@ -503,10 +454,6 @@ namespace places { case mozIPlacesAutoComplete::MATCH_ANYWHERE: case mozIPlacesAutoComplete::MATCH_ANYWHERE_UNMODIFIED: return findAnywhere; - case mozIPlacesAutoComplete::MATCH_BEGINNING: - return findBeginning; - case mozIPlacesAutoComplete::MATCH_BEGINNING_CASE_SENSITIVE: - return findBeginningCaseSensitive; case mozIPlacesAutoComplete::MATCH_BOUNDARY: default: return findOnBoundary; diff --git a/toolkit/components/places/UnifiedComplete.js b/toolkit/components/places/UnifiedComplete.js index d928ff5d3725..b196a387518d 100644 --- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -10,16 +10,6 @@ const MS_PER_DAY = 86400000; // 24 * 60 * 60 * 1000 -// Match type constants. -// These indicate what type of search function we should be using. -const { - MATCH_ANYWHERE, - MATCH_BOUNDARY_ANYWHERE, - MATCH_BOUNDARY, - MATCH_BEGINNING, - MATCH_BEGINNING_CASE_SENSITIVE, -} = Ci.mozIPlacesAutoComplete; - // AutoComplete query type constants. // Describes the various types of queries that we can process rows for. const QUERYTYPE_FILTERED = 0; @@ -576,7 +566,7 @@ function Search(searchString, searchParam, autocompleteListener, this._searchString = Services.textToSubURI.unEscapeURIForUI("UTF-8", suffix); this._strippedPrefix = prefix.toLowerCase(); - this._matchBehavior = UrlbarPrefs.get("matchBehavior"); + this._matchBehavior = Ci.mozIPlacesAutoComplete.MATCH_BOUNDARY; // Set the default behavior for this search. this._behavior = this._searchString ? UrlbarPrefs.get("defaultBehavior") : UrlbarPrefs.get("emptySearchDefaultBehavior"); @@ -982,14 +972,12 @@ Search.prototype = { this._matchAboutPages(); - // If we do not have enough results, and our match type is - // MATCH_BOUNDARY_ANYWHERE, search again with MATCH_ANYWHERE to get more - // results. + // If we do not have enough matches search again with MATCH_ANYWHERE, to + // get more matches. let count = this._counts[UrlbarUtils.MATCH_GROUP.GENERAL] + this._counts[UrlbarUtils.MATCH_GROUP.HEURISTIC]; - if (this._matchBehavior == MATCH_BOUNDARY_ANYWHERE && - count < UrlbarPrefs.get("maxRichResults")) { - this._matchBehavior = MATCH_ANYWHERE; + if (count < UrlbarPrefs.get("maxRichResults")) { + this._matchBehavior = Ci.mozIPlacesAutoComplete.MATCH_ANYWHERE; for (let [query, params] of [ this._adaptiveQuery, this._searchQuery ]) { await conn.executeCached(query, params, this._onResultRow.bind(this)); diff --git a/toolkit/components/places/mozIPlacesAutoComplete.idl b/toolkit/components/places/mozIPlacesAutoComplete.idl index e6732ad89836..7e1370f3555c 100644 --- a/toolkit/components/places/mozIPlacesAutoComplete.idl +++ b/toolkit/components/places/mozIPlacesAutoComplete.idl @@ -19,6 +19,9 @@ interface mozIPlacesAutoComplete : nsISupports ////////////////////////////////////////////////////////////////////////////// //// Matching Constants + // A few of these are not used in Firefox, but are still referenced in + // comm-central. + /** * Match anywhere in each searchable term. */ diff --git a/toolkit/components/places/tests/history/test_update.js b/toolkit/components/places/tests/history/test_update.js index 1d41b01cb418..120286bd9f87 100644 --- a/toolkit/components/places/tests/history/test_update.js +++ b/toolkit/components/places/tests/history/test_update.js @@ -18,6 +18,13 @@ add_task(async function test_error_cases() { /Error: PageInfo: Input should be/, "passing a null as pageInfo should throw an Error" ); + Assert.throws( + () => PlacesUtils.history.update({ + description: "Test description", + }), + /Error: PageInfo: The following properties were expected: url, guid/, + "not included a url or a guid should throw" + ); Assert.throws( () => PlacesUtils.history.update({url: "not a valid url string"}), /Error: PageInfo: Invalid value for property/, @@ -155,7 +162,7 @@ add_task(async function test_previewImageURL_change_saved() { let guid = await PlacesTestUtils.fieldInDB(TEST_URL, "guid"); previewImageURL = IMAGE_URL; - await PlacesUtils.history.update({ url: TEST_URL, guid, previewImageURL }); + await PlacesUtils.history.update({ guid, previewImageURL }); previewImageURLInDB = await PlacesTestUtils.fieldInDB(TEST_URL, "preview_image_url"); Assert.equal(previewImageURL, previewImageURLInDB, "previewImageURL should be updated via GUID as expected"); diff --git a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js index b0d6a072d8e5..c40483b39b67 100644 --- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js +++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js @@ -319,13 +319,15 @@ var addBookmark = async function(aBookmarkObj) { if (aBookmarkObj.keyword) { await PlacesUtils.keywords.insert({ keyword: aBookmarkObj.keyword, - url: aBookmarkObj.uri.spec ? aBookmarkObj.uri.spec : aBookmarkObj.uri, + url: aBookmarkObj.uri instanceof Ci.nsIURI ? aBookmarkObj.uri.spec : aBookmarkObj.uri, postData: aBookmarkObj.postData, }); } if (aBookmarkObj.tags) { - PlacesUtils.tagging.tagURI(aBookmarkObj.uri, aBookmarkObj.tags); + let uri = aBookmarkObj.uri instanceof Ci.nsIURI ? + aBookmarkObj.uri : Services.io.newURI(aBookmarkObj.uri); + PlacesUtils.tagging.tagURI(uri, aBookmarkObj.tags); } }; diff --git a/toolkit/components/places/tests/unifiedcomplete/test_match_beginning.js b/toolkit/components/places/tests/unifiedcomplete/test_match_beginning.js deleted file mode 100644 index 5711a8c552c5..000000000000 --- a/toolkit/components/places/tests/unifiedcomplete/test_match_beginning.js +++ /dev/null @@ -1,54 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * 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/. */ - -/** - * Test bug 451760 which allows matching only at the beginning of urls or - * titles to simulate Firefox 2 functionality. - */ - -add_task(async function test_match_beginning() { - Services.prefs.setBoolPref("browser.urlbar.autoFill", false); - - let uri1 = NetUtil.newURI("http://x.com/y"); - let uri2 = NetUtil.newURI("https://y.com/x"); - await PlacesTestUtils.addVisits([ - { uri: uri1, title: "a b" }, - { uri: uri2, title: "b a" }, - ]); - - info("Match at the beginning of titles"); - Services.prefs.setIntPref("browser.urlbar.matchBehavior", 3); - await check_autocomplete({ - search: "a", - matches: [ { uri: uri1, title: "a b" } ], - }); - - info("Match at the beginning of titles"); - await check_autocomplete({ - search: "b", - matches: [ { uri: uri2, title: "b a" } ], - }); - - info("Match at the beginning of urls"); - await check_autocomplete({ - search: "x", - matches: [ { uri: uri1, title: "a b" } ], - }); - - info("Match at the beginning of urls"); - await check_autocomplete({ - search: "y", - matches: [ { uri: uri2, title: "b a" } ], - }); - - info("Sanity check that matching anywhere finds more"); - Services.prefs.setIntPref("browser.urlbar.matchBehavior", 1); - await check_autocomplete({ - search: "a", - matches: [ { uri: uri1, title: "a b" }, - { uri: uri2, title: "b a" } ], - }); - - await cleanup(); -}); diff --git a/toolkit/components/places/tests/unifiedcomplete/test_word_boundary_search.js b/toolkit/components/places/tests/unifiedcomplete/test_word_boundary_search.js index 7fc2f9d8cf76..150c336f6ef2 100644 --- a/toolkit/components/places/tests/unifiedcomplete/test_word_boundary_search.js +++ b/toolkit/components/places/tests/unifiedcomplete/test_word_boundary_search.js @@ -3,15 +3,12 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ /** - * Test bug 393678 to make sure matches against the url, title, tags are only - * made on word boundaries instead of in the middle of words. + * Test to make sure matches against the url, title, tags are first made on word + * boundaries, instead of in the middle of words, and later are extended to the + * whole words. For this test it is critical to check sorting of the matches. * * Make sure we don't try matching one after a CamelCase because the upper-case * isn't really a word boundary. (bug 429498) - * - * Bug 429531 provides switching between "must match on word boundary" and "can - * match," so leverage "must match" pref for checking word boundary logic and - * make sure "can match" matches anywhere. */ var katakana = ["\u30a8", "\u30c9"]; // E, Do @@ -21,155 +18,179 @@ add_task(async function test_escape() { Services.prefs.setBoolPref("browser.urlbar.autoFill.searchEngines", false); Services.prefs.setBoolPref("browser.urlbar.autoFill", false); - let uri1 = NetUtil.newURI("http://matchme/"); - let uri2 = NetUtil.newURI("http://dontmatchme/"); - let uri3 = NetUtil.newURI("http://title/1"); - let uri4 = NetUtil.newURI("http://title/2"); - let uri5 = NetUtil.newURI("http://tag/1"); - let uri6 = NetUtil.newURI("http://tag/2"); - let uri7 = NetUtil.newURI("http://crazytitle/"); - let uri8 = NetUtil.newURI("http://katakana/"); - let uri9 = NetUtil.newURI("http://ideograph/"); - let uri10 = NetUtil.newURI("http://camel/pleaseMatchMe/"); - await PlacesTestUtils.addVisits([ - { uri: uri1, title: "title1" }, - { uri: uri2, title: "title1" }, - { uri: uri3, title: "matchme2" }, - { uri: uri4, title: "dontmatchme3" }, - { uri: uri5, title: "title1" }, - { uri: uri6, title: "title1" }, - { uri: uri7, title: "!@#$%^&*()_+{}|:<>?word" }, - { uri: uri8, title: katakana.join("") }, - { uri: uri9, title: ideograph.join("") }, - { uri: uri10, title: "title1" }, + { uri: "http://matchme/", title: "title1" }, + { uri: "http://dontmatchme/", title: "title1" }, + { uri: "http://title/1", title: "matchme2" }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://tag/1", title: "title1" }, + { uri: "http://tag/2", title: "title1" }, + { uri: "http://crazytitle/", title: "!@#$%^&*()_+{}|:<>?word" }, + { uri: "http://katakana/", title: katakana.join("") }, + { uri: "http://ideograph/", title: ideograph.join("") }, + { uri: "http://camel/pleaseMatchMe/", title: "title1" }, ]); - await addBookmark( { uri: uri5, title: "title1", tags: [ "matchme2" ] } ); - await addBookmark( { uri: uri6, title: "title1", tags: [ "dontmatchme3" ] } ); - - // match only on word boundaries - Services.prefs.setIntPref("browser.urlbar.matchBehavior", 2); + await addBookmark( { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ] } ); + await addBookmark( { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ] } ); info("Match 'match' at the beginning or after / or on a CamelCase"); await check_autocomplete({ search: "match", - matches: [ { uri: uri1, title: "title1" }, - { uri: uri3, title: "matchme2" }, - { uri: uri5, title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, - { uri: uri10, title: "title1" } ], + checkSorting: true, + matches: [ + { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, + { uri: "http://camel/pleaseMatchMe/", title: "title1" }, + { uri: "http://title/1", title: "matchme2" }, + { uri: "http://matchme/", title: "title1" }, + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://dontmatchme/", title: "title1" }, + ], }); info("Match 'dont' at the beginning or after /"); await check_autocomplete({ search: "dont", - matches: [ { uri: uri2, title: "title1" }, - { uri: uri4, title: "dontmatchme3" }, - { uri: uri6, title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] } ], + checkSorting: true, + matches: [ + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://dontmatchme/", title: "title1" }, + ], }); info("Match 'match' at the beginning or after / or on a CamelCase"); await check_autocomplete({ search: "2", - matches: [ { uri: uri3, title: "matchme2" }, - { uri: uri4, title: "dontmatchme3" }, - { uri: uri5, title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, - { uri: uri6, title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] } ], + checkSorting: true, + matches: [ + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://title/1", title: "matchme2" }, + ], }); info("Match 't' at the beginning or after /"); await check_autocomplete({ search: "t", - matches: [ { uri: uri1, title: "title1" }, - { uri: uri2, title: "title1" }, - { uri: uri3, title: "matchme2" }, - { uri: uri4, title: "dontmatchme3" }, - { uri: uri5, title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, - { uri: uri6, title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, - { uri: uri10, title: "title1" } ], + checkSorting: true, + matches: [ + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, + { uri: "http://camel/pleaseMatchMe/", title: "title1" }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://title/1", title: "matchme2" }, + { uri: "http://dontmatchme/", title: "title1" }, + { uri: "http://matchme/", title: "title1" }, + { uri: "http://katakana/", title: katakana.join("") }, + { uri: "http://crazytitle/", title: "!@#$%^&*()_+{}|:<>?word" }, + ], }); info("Match 'word' after many consecutive word boundaries"); await check_autocomplete({ search: "word", - matches: [ { uri: uri7, title: "!@#$%^&*()_+{}|:<>?word" } ], + checkSorting: true, + matches: [ + { uri: "http://crazytitle/", title: "!@#$%^&*()_+{}|:<>?word" }, + ], }); info("Match a word boundary '/' for everything"); await check_autocomplete({ search: "/", - matches: [ { uri: uri1, title: "title1" }, - { uri: uri2, title: "title1" }, - { uri: uri3, title: "matchme2" }, - { uri: uri4, title: "dontmatchme3" }, - { uri: uri5, title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, - { uri: uri6, title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, - { uri: uri7, title: "!@#$%^&*()_+{}|:<>?word" }, - { uri: uri8, title: katakana.join("") }, - { uri: uri9, title: ideograph.join("") }, - { uri: uri10, title: "title1" } ], + checkSorting: true, + matches: [ + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, + { uri: "http://camel/pleaseMatchMe/", title: "title1" }, + { uri: "http://ideograph/", title: ideograph.join("") }, + { uri: "http://katakana/", title: katakana.join("") }, + { uri: "http://crazytitle/", title: "!@#$%^&*()_+{}|:<>?word" }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://title/1", title: "matchme2" }, + { uri: "http://dontmatchme/", title: "title1" }, + { uri: "http://matchme/", title: "title1" }, + ], }); info("Match word boundaries '()_+' that are among word boundaries"); await check_autocomplete({ search: "()_+", - matches: [ { uri: uri7, title: "!@#$%^&*()_+{}|:<>?word" } ], + checkSorting: true, + matches: [ + { uri: "http://crazytitle/", title: "!@#$%^&*()_+{}|:<>?word" }, + ], }); info("Katakana characters form a string, so match the beginning"); await check_autocomplete({ search: katakana[0], - matches: [ { uri: uri8, title: katakana.join("") } ], + checkSorting: true, + matches: [ + { uri: "http://katakana/", title: katakana.join("") }, + ], }); /* - do_print("Middle of a katakana word shouldn't be matched"); - yield check_autocomplete({ + info("Middle of a katakana word shouldn't be matched"); + await check_autocomplete({ search: katakana[1], - matches: [ ] + matches: [ ], }); */ + info("Ideographs are treated as words so 'nin' is one word"); await check_autocomplete({ search: ideograph[0], - matches: [ { uri: uri9, title: ideograph.join("") } ], + checkSorting: true, + matches: [ { uri: "http://ideograph/", title: ideograph.join("") } ], }); info("Ideographs are treated as words so 'ten' is another word"); await check_autocomplete({ search: ideograph[1], - matches: [ { uri: uri9, title: ideograph.join("") } ], + checkSorting: true, + matches: [ { uri: "http://ideograph/", title: ideograph.join("") } ], }); info("Ideographs are treated as words so 'do' is yet another word"); await check_autocomplete({ search: ideograph[2], - matches: [ { uri: uri9, title: ideograph.join("") } ], + checkSorting: true, + matches: [ { uri: "http://ideograph/", title: ideograph.join("") } ], }); - info("Extra negative assert that we don't match in the middle"); + info("Match in the middle. Should just be sorted by frecency."); await check_autocomplete({ search: "ch", - matches: [ ], + checkSorting: true, + matches: [ + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, + { uri: "http://camel/pleaseMatchMe/", title: "title1" }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://title/1", title: "matchme2" }, + { uri: "http://dontmatchme/", title: "title1" }, + { uri: "http://matchme/", title: "title1" }, + ], }); - info("Don't match one character after a camel-case word boundary (bug 429498)"); + // Also this test should just be sorted by frecency. + info("Don't match one character after a camel-case word boundary (bug 429498). Should just be sorted by frecency."); await check_autocomplete({ search: "atch", - matches: [ ], - }); - - // match against word boundaries and anywhere - Services.prefs.setIntPref("browser.urlbar.matchBehavior", 1); - - await check_autocomplete({ - search: "tch", - matches: [ { uri: uri1, title: "title1" }, - { uri: uri2, title: "title1" }, - { uri: uri3, title: "matchme2" }, - { uri: uri4, title: "dontmatchme3" }, - { uri: uri5, title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, - { uri: uri6, title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, - { uri: uri10, title: "title1" } ], + checkSorting: true, + matches: [ + { uri: "http://tag/2", title: "title1", tags: [ "dontmatchme3" ], style: [ "bookmark-tag" ] }, + { uri: "http://tag/1", title: "title1", tags: [ "matchme2" ], style: [ "bookmark-tag" ] }, + { uri: "http://camel/pleaseMatchMe/", title: "title1" }, + { uri: "http://title/2", title: "dontmatchme3" }, + { uri: "http://title/1", title: "matchme2" }, + { uri: "http://dontmatchme/", title: "title1" }, + { uri: "http://matchme/", title: "title1" }, + ], }); await cleanup(); diff --git a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini index e8b1dc359f83..31d67fa8ccdf 100644 --- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini +++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini @@ -40,7 +40,6 @@ support-files = [test_keyword_search.js] [test_keyword_search_actions.js] [test_keywords.js] -[test_match_beginning.js] [test_multi_word_search.js] [test_PlacesSearchAutocompleteProvider.js] skip-if = appname == "thunderbird"