From c471ec25c819555a0bd704632aed894503cae9e5 Mon Sep 17 00:00:00 2001 From: Gavin Sharp Date: Sat, 14 Dec 2013 16:10:52 -0800 Subject: [PATCH 01/18] Bug 950405: add a temporary whatsnew page on Nightly for Australis survey, r=Unfocused --- browser/components/nsBrowserContentHandler.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/browser/components/nsBrowserContentHandler.js b/browser/components/nsBrowserContentHandler.js index 466950c1d7c9..c6685a14d26d 100644 --- a/browser/components/nsBrowserContentHandler.js +++ b/browser/components/nsBrowserContentHandler.js @@ -583,6 +583,25 @@ nsBrowserContentHandler.prototype = { overridePage = overridePage.replace("%OLD_VERSION%", old_mstone); break; + + // Temporary case for Australis whatsnew + case OVERRIDE_NEW_BUILD_ID: + let locale = "en-US"; + try { + locale = Services.prefs.getCharPref("general.useragent.locale"); + } catch (e) {} + + let showedAustralisWhatsNew = false; + try { + showedAustralisWhatsNew = Services.prefs.getBoolPref("browser.showedAustralisWhatsNew"); + } catch(e) {} + + // Show the Australis whatsnew page for en-US if we haven't yet shown it + if (!showedAustralisWhatsNew && locale == "en-US") { + Services.prefs.setBoolPref("browser.showedAustralisWhatsNew", true); + overridePage = "https://www.mozilla.org/en-US/firefox/29.0a1/whatsnew/"; + } + break; } } } catch (ex) {} From 856bcca684c8c4a1568fb72795558e0ab6afb855 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Wed, 18 Dec 2013 14:17:00 +0000 Subject: [PATCH 02/18] Bug 948985 - ensure Australis' non-removable API-based widgets with a defaultArea work after destruction, r=Unfocused --HG-- extra : rebase_source : 2f994be1ff635b6871ed45f0185edcae8141cdcf --- .../customizableui/src/CustomizableUI.jsm | 5 ++- .../customizableui/test/browser.ini | 1 + ...rowser_948985_non_removable_defaultArea.js | 32 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 browser/components/customizableui/test/browser_948985_non_removable_defaultArea.js diff --git a/browser/components/customizableui/src/CustomizableUI.jsm b/browser/components/customizableui/src/CustomizableUI.jsm index 6f086d9cd162..6e0cb152e7ec 100644 --- a/browser/components/customizableui/src/CustomizableUI.jsm +++ b/browser/components/customizableui/src/CustomizableUI.jsm @@ -1695,7 +1695,10 @@ let CustomizableUIInternal = { // If the widget doesn't have an existing placement, and it hasn't been // seen before, then add it to its default area so it can be used. - if (autoAdd && !widget.currentArea && !gSeenWidgets.has(widget.id)) { + // If the widget is not removable, we *have* to add it to its default + // area here. + let canBeAutoAdded = autoAdd && !gSeenWidgets.has(widget.id); + if (!widget.currentArea && (!widget.removable || canBeAutoAdded)) { this.beginBatchUpdate(); try { gSeenWidgets.add(widget.id); diff --git a/browser/components/customizableui/test/browser.ini b/browser/components/customizableui/test/browser.ini index 3a7dfad508bf..e80ed076dbc1 100644 --- a/browser/components/customizableui/test/browser.ini +++ b/browser/components/customizableui/test/browser.ini @@ -48,4 +48,5 @@ skip-if = os == "mac" [browser_944887_destroyWidget_should_destroy_in_palette.js] [browser_945739_showInPrivateBrowsing_customize_mode.js] [browser_947987_removable_default.js] +[browser_948985_non_removable_defaultArea.js] [browser_panel_toggle.js] diff --git a/browser/components/customizableui/test/browser_948985_non_removable_defaultArea.js b/browser/components/customizableui/test/browser_948985_non_removable_defaultArea.js new file mode 100644 index 000000000000..456c9ed02a97 --- /dev/null +++ b/browser/components/customizableui/test/browser_948985_non_removable_defaultArea.js @@ -0,0 +1,32 @@ +/* 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/. */ + +const kWidgetId = "test-destroy-non-removable-defaultArea"; + +add_task(function() { + let spec = {id: kWidgetId, label: "Test non-removable defaultArea re-adding.", + removable: false, defaultArea: CustomizableUI.AREA_NAVBAR}; + CustomizableUI.createWidget(spec); + let placement = CustomizableUI.getPlacementOfWidget(kWidgetId); + ok(placement, "Should have placed the widget."); + is(placement && placement.area, CustomizableUI.AREA_NAVBAR, "Widget should be in navbar"); + CustomizableUI.destroyWidget(kWidgetId); + CustomizableUI.removeWidgetFromArea(kWidgetId); + + CustomizableUI.createWidget(spec); + ok(placement, "Should have placed the widget."); + is(placement && placement.area, CustomizableUI.AREA_NAVBAR, "Widget should be in navbar"); + CustomizableUI.destroyWidget(kWidgetId); + CustomizableUI.removeWidgetFromArea(kWidgetId); + + const kPrefCustomizationAutoAdd = "browser.uiCustomization.autoAdd"; + Services.prefs.setBoolPref(kPrefCustomizationAutoAdd, false); + CustomizableUI.createWidget(spec); + ok(placement, "Should have placed the widget."); + is(placement && placement.area, CustomizableUI.AREA_NAVBAR, "Widget should be in navbar"); + CustomizableUI.destroyWidget(kWidgetId); + CustomizableUI.removeWidgetFromArea(kWidgetId); + Services.prefs.clearUserPref(kPrefCustomizationAutoAdd); +}); + From a6b8d5be6d77b6ec7297a2c560423ac4dfa8d955 Mon Sep 17 00:00:00 2001 From: Scott Johnson Date: Tue, 1 Oct 2013 14:52:13 -0500 Subject: [PATCH 03/18] Bug 878935, Part 2: Pause painting while reflow-on-zoom is in progress to provide a better user experience. [r=kats,dbaron] --- docshell/base/nsIMarkupDocumentViewer.idl | 14 ++- layout/base/nsDocumentViewer.cpp | 42 ++++++- layout/base/nsIPresShell.h | 21 +++- layout/base/nsPresShell.cpp | 29 +++++ layout/base/nsPresShell.h | 3 + mobile/android/chrome/content/browser.js | 130 ++++++++++++++++------ 6 files changed, 198 insertions(+), 41 deletions(-) diff --git a/docshell/base/nsIMarkupDocumentViewer.idl b/docshell/base/nsIMarkupDocumentViewer.idl index 342f741435c1..09c255b583f7 100644 --- a/docshell/base/nsIMarkupDocumentViewer.idl +++ b/docshell/base/nsIMarkupDocumentViewer.idl @@ -23,7 +23,7 @@ interface nsIMarkupDocumentViewer; [ref] native nsIMarkupDocumentViewerTArray(nsTArray >); -[scriptable, uuid(3528324f-f5d3-4724-bd8d-9233a7114112)] +[scriptable, uuid(7aea9561-5346-401c-b40e-418688da2d0d)] interface nsIMarkupDocumentViewer : nsISupports { @@ -82,6 +82,18 @@ interface nsIMarkupDocumentViewer : nsISupports */ void changeMaxLineBoxWidth(in int32_t maxLineBoxWidth); + /** + * Instruct the refresh driver to discontinue painting until further + * notice. + */ + void pausePainting(); + + /** + * Instruct the refresh driver to resume painting after a previous call to + * pausePainting(). + */ + void resumePainting(); + /* * Render the document as if being viewed on a device with the specified * media type. This will cause a reflow. diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 885c8444d2da..ee428a0919fb 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -2700,6 +2700,17 @@ struct LineBoxInfo nscoord mMaxLineBoxWidth; }; +static void +ChangeChildPaintingEnabled(nsIMarkupDocumentViewer* aChild, void* aClosure) +{ + bool* enablePainting = (bool*) aClosure; + if (*enablePainting) { + aChild->ResumePainting(); + } else { + aChild->PausePainting(); + } +} + static void ChangeChildMaxLineBoxWidth(nsIMarkupDocumentViewer* aChild, void* aClosure) { @@ -3126,7 +3137,36 @@ NS_IMETHODIMP nsDocumentViewer::AppendSubtree(nsTArrayPausePainting(); + } + + return NS_OK; +} + +NS_IMETHODIMP +nsDocumentViewer::ResumePainting() +{ + bool enablePaint = true; + CallChildren(ChangeChildPaintingEnabled, &enablePaint); + + nsIPresShell* presShell = GetPresShell(); + if (presShell) { + presShell->ResumePainting(); + } + + return NS_OK; +} + +NS_IMETHODIMP +nsDocumentViewer::ChangeMaxLineBoxWidth(int32_t aMaxLineBoxWidth) { // Change the max line box width for all children. struct LineBoxInfo lbi = { aMaxLineBoxWidth }; diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index 30d4f6025165..db9470f7b921 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -129,10 +129,10 @@ typedef struct CapturingContentInfo { } CapturingContentInfo; -// f5b542a9-eaf0-4560-a656-37a9d379864c +// 0e4f2b36-7ab8-43c5-b912-5c311566297c #define NS_IPRESSHELL_IID \ -{ 0xf5b542a9, 0xeaf0, 0x4560, \ - { 0x37, 0xa9, 0xd3, 0x79, 0x86, 0x4c } } +{ 0xde498c49, 0xf83f, 0x47bf, \ + {0x8c, 0xc6, 0x8f, 0xf8, 0x74, 0x62, 0x22, 0x23 } } // debug VerifyReflow flags #define VERIFY_REFLOW_ON 0x01 @@ -836,6 +836,20 @@ public: */ bool IsPaintingSuppressed() const { return mPaintingSuppressed; } + /** + * Pause painting by freezing the refresh driver of this and all parent + * presentations. This may not have the desired effect if this pres shell + * has its own refresh driver. + */ + virtual void PausePainting() = 0; + + /** + * Resume painting by thawing the refresh driver of this and all parent + * presentations. This may not have the desired effect if this pres shell + * has its own refresh driver. + */ + virtual void ResumePainting() = 0; + /** * Unsuppress painting. */ @@ -1601,6 +1615,7 @@ protected: bool mFontSizeInflationForceEnabled; bool mFontSizeInflationDisabledInMasterProcess; bool mFontSizeInflationEnabled; + bool mPaintingIsFrozen; // Dirty bit indicating that mFontSizeInflationEnabled needs to be recomputed. bool mFontSizeInflationEnabledIsDirty; diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index a600299c2b9f..b578f6d1670c 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -724,6 +724,8 @@ PresShell::PresShell() "layout.reflow.synthMouseMove", true); addedSynthMouseMove = true; } + + mPaintingIsFrozen = false; } NS_IMPL_ISUPPORTS7(PresShell, nsIPresShell, nsIDocumentObserver, @@ -744,6 +746,13 @@ PresShell::~PresShell() mLastCallbackEventRequest == nullptr, "post-reflow queues not empty. This means we're leaking"); + // Verify that if painting was frozen, but we're being removed from the tree, + // that we now re-enable painting on our refresh driver, since it may need to + // be re-used by another presentation. + if (mPaintingIsFrozen) { + mPresContext->RefreshDriver()->Thaw(); + } + #ifdef DEBUG MOZ_ASSERT(mPresArenaAllocCount == 0, "Some pres arena objects were not freed"); @@ -9931,3 +9940,23 @@ nsIPresShell::SetMaxLineBoxWidth(nscoord aMaxLineBoxWidth) FrameNeedsReflow(GetRootFrame(), eResize, NS_FRAME_HAS_DIRTY_CHILDREN); } } + +void +PresShell::PausePainting() +{ + if (GetPresContext()->RefreshDriver()->PresContext() != GetPresContext()) + return; + + mPaintingIsFrozen = true; + GetPresContext()->RefreshDriver()->Freeze(); +} + +void +PresShell::ResumePainting() +{ + if (GetPresContext()->RefreshDriver()->PresContext() != GetPresContext()) + return; + + mPaintingIsFrozen = false; + GetPresContext()->RefreshDriver()->Thaw(); +} diff --git a/layout/base/nsPresShell.h b/layout/base/nsPresShell.h index 8e14e4d8eef7..932f9662bb75 100644 --- a/layout/base/nsPresShell.h +++ b/layout/base/nsPresShell.h @@ -689,6 +689,9 @@ protected: virtual void ThemeChanged() MOZ_OVERRIDE { mPresContext->ThemeChanged(); } virtual void BackingScaleFactorChanged() MOZ_OVERRIDE { mPresContext->UIResolutionChanged(); } + virtual void PausePainting() MOZ_OVERRIDE; + virtual void ResumePainting() MOZ_OVERRIDE; + void UpdateImageVisibility(); nsRevocableEventPtr > mUpdateImageVisibilityEvent; diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 6e6b640b19a4..32e4b872992d 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -182,6 +182,11 @@ function doChangeMaxLineBoxWidth(aWidth) { if (range) { BrowserEventHandler._zoomInAndSnapToRange(range); + } else { + // In this case, we actually didn't zoom into a specific range. It probably + // happened from a page load reflow-on-zoom event, so we need to make sure + // painting is re-enabled. + BrowserApp.selectedTab.clearReflowOnZoomPendingActions(); } } @@ -2744,6 +2749,7 @@ Tab.prototype = { this.browser.addEventListener("MozApplicationManifest", this, true); Services.obs.addObserver(this, "before-first-paint", false); + Services.obs.addObserver(this, "after-viewport-change", false); Services.prefs.addObserver("browser.ui.zoom.force-user-scalable", this, false); if (aParams.delayLoad) { @@ -2803,21 +2809,58 @@ Tab.prototype = { return minFontSize / this.getInflatedFontSizeFor(aElement); }, + clearReflowOnZoomPendingActions: function() { + // Reflow was completed, so now re-enable painting. + let webNav = BrowserApp.selectedTab.window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation); + let docShell = webNav.QueryInterface(Ci.nsIDocShell); + let docViewer = docShell.contentViewer.QueryInterface(Ci.nsIMarkupDocumentViewer); + docViewer.resumePainting(); + + BrowserApp.selectedTab._mReflozPositioned = false; + }, + + /** + * Reflow on zoom consists of a few different sub-operations: + * + * 1. When a double-tap event is seen, we verify that the correct preferences + * are enabled and perform the pre-position handling calculation. We also + * signal that reflow-on-zoom should be performed at this time, and pause + * painting. + * 2. During the next call to setViewport(), which is in the Tab prototype, + * we detect that a call to changeMaxLineBoxWidth should be performed. If + * we're zooming out, then the max line box width should be reset at this + * time. Otherwise, we call performReflowOnZoom. + * 2a. PerformReflowOnZoom() and resetMaxLineBoxWidth() schedule a call to + * doChangeMaxLineBoxWidth, based on a timeout specified in preferences. + * 3. doChangeMaxLineBoxWidth changes the line box width (which also + * schedules a reflow event), and then calls _zoomInAndSnapToRange. + * 4. _zoomInAndSnapToRange performs the positioning of reflow-on-zoom and + * then re-enables painting. + * + * Some of the events happen synchronously, while others happen asynchronously. + * The following is a rough sketch of the progression of events: + * + * double tap event seen -> onDoubleTap() -> ... asynchronous ... + * -> setViewport() -> performReflowOnZoom() -> ... asynchronous ... + * -> doChangeMaxLineBoxWidth() -> _zoomInAndSnapToRange() + * -> ... asynchronous ... -> setViewport() -> Observe('after-viewport-change') + * -> resumePainting() + */ performReflowOnZoom: function(aViewport) { - let zoom = this._drawZoom ? this._drawZoom : aViewport.zoom; + let zoom = this._drawZoom ? this._drawZoom : aViewport.zoom; - let viewportWidth = gScreenWidth / zoom; - let reflozTimeout = Services.prefs.getIntPref("browser.zoom.reflowZoom.reflowTimeout"); + let viewportWidth = gScreenWidth / zoom; + let reflozTimeout = Services.prefs.getIntPref("browser.zoom.reflowZoom.reflowTimeout"); - if (gReflowPending) { - clearTimeout(gReflowPending); - } + if (gReflowPending) { + clearTimeout(gReflowPending); + } - // We add in a bit of fudge just so that the end characters - // don't accidentally get clipped. 15px is an arbitrary choice. - gReflowPending = setTimeout(doChangeMaxLineBoxWidth, - reflozTimeout, - viewportWidth - 15); + // We add in a bit of fudge just so that the end characters + // don't accidentally get clipped. 15px is an arbitrary choice. + gReflowPending = setTimeout(doChangeMaxLineBoxWidth, + reflozTimeout, + viewportWidth - 15); }, /** @@ -2889,6 +2932,7 @@ Tab.prototype = { this.browser.removeEventListener("MozApplicationManifest", this, true); Services.obs.removeObserver(this, "before-first-paint"); + Services.obs.removeObserver(this, "after-viewport-change"); Services.prefs.removeObserver("browser.ui.zoom.force-user-scalable", this); // Make sure the previously selected panel remains selected. The selected panel of a deck is @@ -3175,6 +3219,7 @@ Tab.prototype = { // In this case, the user pinch-zoomed in, so we don't want to // preserve position as we would with reflow-on-zoom. BrowserApp.selectedTab.probablyNeedRefloz = false; + BrowserApp.selectedTab.clearReflowOnZoomPendingActions(); BrowserApp.selectedTab._mReflozPoint = null; } @@ -4166,6 +4211,11 @@ Tab.prototype = { BrowserApp.selectedTab.performReflowOnZoom(vp); } break; + case "after-viewport-change": + if (BrowserApp.selectedTab._mReflozPositioned) { + BrowserApp.selectedTab.clearReflowOnZoomPendingActions(); + } + break; case "nsPref:changed": if (aData == "browser.ui.zoom.force-user-scalable") ViewportHandler.updateMetadata(this, false); @@ -4484,13 +4534,23 @@ var BrowserEventHandler = { if (BrowserEventHandler.mReflozPref && !BrowserApp.selectedTab._mReflozPoint && !this._shouldSuppressReflowOnZoom(element)) { - let data = JSON.parse(aData); - let zoomPointX = data.x; - let zoomPointY = data.y; - BrowserApp.selectedTab._mReflozPoint = { x: zoomPointX, y: zoomPointY, - range: BrowserApp.selectedBrowser.contentDocument.caretPositionFromPoint(zoomPointX, zoomPointY) }; - BrowserApp.selectedTab.probablyNeedRefloz = true; + // See comment above performReflowOnZoom() for a detailed description of + // the events happening in the reflow-on-zoom operation. + let data = JSON.parse(aData); + let zoomPointX = data.x; + let zoomPointY = data.y; + + BrowserApp.selectedTab._mReflozPoint = { x: zoomPointX, y: zoomPointY, + range: BrowserApp.selectedBrowser.contentDocument.caretPositionFromPoint(zoomPointX, zoomPointY) }; + + // Before we perform a reflow on zoom, let's disable painting. + let webNav = BrowserApp.selectedTab.window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation); + let docShell = webNav.QueryInterface(Ci.nsIDocShell); + let docViewer = docShell.contentViewer.QueryInterface(Ci.nsIMarkupDocumentViewer); + docViewer.pausePainting(); + + BrowserApp.selectedTab.probablyNeedRefloz = true; } if (!element) { @@ -4590,11 +4650,7 @@ var BrowserEventHandler = { }, _zoomInAndSnapToRange: function(aRange) { - if (!aRange) { - Cu.reportError("aRange is null in zoomInAndSnapToRange. Unable to maintain position."); - return; - } - + // aRange is always non-null here, since a check happened previously. let viewport = BrowserApp.selectedTab.getViewport(); let fudge = 15; // Add a bit of fudge. let boundingElement = aRange.offsetNode; @@ -4617,6 +4673,8 @@ var BrowserEventHandler = { let leftAdjustment = parseInt(boundingStyle.paddingLeft) + parseInt(boundingStyle.borderLeftWidth); + BrowserApp.selectedTab._mReflozPositioned = true; + rect.type = "Browser:ZoomToRect"; rect.x = Math.max(viewport.cssPageLeft, rect.x - fudge + leftAdjustment); rect.y = Math.max(topPos, viewport.cssPageTop); @@ -4625,22 +4683,22 @@ var BrowserEventHandler = { sendMessageToJava(rect); BrowserApp.selectedTab._mReflozPoint = null; - }, + }, - onPinchFinish: function(aData) { - let data = {}; - try { - data = JSON.parse(aData); - } catch(ex) { - console.log(ex); - return; - } + onPinchFinish: function(aData) { + let data = {}; + try { + data = JSON.parse(aData); + } catch(ex) { + console.log(ex); + return; + } - if (BrowserEventHandler.mReflozPref && - data.zoomDelta < 0.0) { - BrowserEventHandler.resetMaxLineBoxWidth(); - } - }, + if (BrowserEventHandler.mReflozPref && + data.zoomDelta < 0.0) { + BrowserEventHandler.resetMaxLineBoxWidth(); + } + }, _shouldZoomToElement: function(aElement) { let win = aElement.ownerDocument.defaultView; From 606384dc264f721262f571e8a97b61244f52f677 Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Wed, 18 Dec 2013 16:26:53 +0100 Subject: [PATCH 04/18] Bug 951675 - Make sure to always copy data from the persistent cache r=yoric From bbe5effc34b6e81e57750ec05164e7e10de75d7a Mon Sep 17 00:00:00 2001 --- .../components/sessionstore/src/TabState.jsm | 45 +++++++++++++++---- browser/components/sessionstore/src/Utils.jsm | 11 +++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/browser/components/sessionstore/src/TabState.jsm b/browser/components/sessionstore/src/TabState.jsm index 3cf7b3bcea8c..e35bf4b5f0d1 100644 --- a/browser/components/sessionstore/src/TabState.jsm +++ b/browser/components/sessionstore/src/TabState.jsm @@ -182,9 +182,6 @@ let TabStateInternal = { tabData.index = history.index; } - // Copy data from the persistent cache. - this._copyFromPersistentCache(tab, tabData); - // If we're still the latest async collection for the given tab and // the cache hasn't been filled by collect() in the meantime, let's // fill the cache with the data we received. @@ -193,6 +190,16 @@ let TabStateInternal = { this._pendingCollections.delete(browser); } + // Copy data from the persistent cache. We need to create an explicit + // copy of the |tabData| object so that the properties injected by + // |_copyFromPersistentCache| don't end up in the non-persistent cache. + // The persistent cache does not store "null" values, so any values that + // have been cleared by the frame script would not be overriden by + // |_copyFromPersistentCache|. These two caches are only an interim + // solution and the non-persistent one will go away soon. + tabData = Utils.copy(tabData); + this._copyFromPersistentCache(tab, tabData); + throw new Task.Result(tabData); }.bind(this)); @@ -219,7 +226,16 @@ let TabStateInternal = { throw new TypeError("Expecting a tab"); } if (TabStateCache.has(tab)) { - return TabStateCache.get(tab); + // Copy data from the persistent cache. We need to create an explicit + // copy of the |tabData| object so that the properties injected by + // |_copyFromPersistentCache| don't end up in the non-persistent cache. + // The persistent cache does not store "null" values, so any values that + // have been cleared by the frame script would not be overriden by + // |_copyFromPersistentCache|. These two caches are only an interim + // solution and the non-persistent one will go away soon. + let tabData = Utils.copy(TabStateCache.get(tab)); + this._copyFromPersistentCache(tab, tabData); + return tabData; } let tabData = this._collectSyncUncached(tab); @@ -228,6 +244,16 @@ let TabStateInternal = { TabStateCache.set(tab, tabData); } + // Copy data from the persistent cache. We need to create an explicit + // copy of the |tabData| object so that the properties injected by + // |_copyFromPersistentCache| don't end up in the non-persistent cache. + // The persistent cache does not store "null" values, so any values that + // have been cleared by the frame script would not be overriden by + // |_copyFromPersistentCache|. These two caches are only an interim + // solution and the non-persistent one will go away soon. + tabData = Utils.copy(tabData); + this._copyFromPersistentCache(tab, tabData); + // Prevent all running asynchronous collections from filling the cache. // Every asynchronous data collection started before a collectSync() call // can't expect to retrieve different data than the sync call. That's why @@ -262,7 +288,13 @@ let TabStateInternal = { * up-to-date. */ clone: function (tab) { - return this._collectSyncUncached(tab, {includePrivateData: true}); + let options = {includePrivateData: true}; + let tabData = this._collectSyncUncached(tab, options); + + // Copy data from the persistent cache. + this._copyFromPersistentCache(tab, tabData, options); + + return tabData; }, /** @@ -305,9 +337,6 @@ let TabStateInternal = { tabData.index = history.index; } - // Copy data from the persistent cache. - this._copyFromPersistentCache(tab, tabData, options); - return tabData; }, diff --git a/browser/components/sessionstore/src/Utils.jsm b/browser/components/sessionstore/src/Utils.jsm index e061cd35b86a..63c3c72da7df 100644 --- a/browser/components/sessionstore/src/Utils.jsm +++ b/browser/components/sessionstore/src/Utils.jsm @@ -64,5 +64,16 @@ this.Utils = Object.freeze({ map.set(otherKey, value); map.delete(key); } + }, + + // Copies all properties of a given object to a new one and returns it. + copy: function (from) { + let to = {}; + + for (let key of Object.keys(from)) { + to[key] = from[key]; + } + + return to; } }); From b83c0e725c3e20dcd977ed8a68b45af4700e3007 Mon Sep 17 00:00:00 2001 From: Albert Juhe Date: Fri, 20 Dec 2013 11:40:21 -0500 Subject: [PATCH 05/18] Bug 950667 - DevTools CSS - Use an attribute instead of .highlighted class for styling tabs like the paused debugger. r=bgrins --- .../test/browser_dbg_on-pause-highlight.js | 6 ++- .../test/browser_dbg_on-pause-raise.js | 9 +++-- .../test/browser_toolbox_highlight.js | 8 ++-- browser/devtools/framework/toolbox.js | 4 +- .../themes/shared/devtools/toolbars.inc.css | 38 +++++++++---------- 5 files changed, 35 insertions(+), 30 deletions(-) diff --git a/browser/devtools/debugger/test/browser_dbg_on-pause-highlight.js b/browser/devtools/debugger/test/browser_dbg_on-pause-highlight.js index 0ca6b4a52afc..891df807315b 100644 --- a/browser/devtools/debugger/test/browser_dbg_on-pause-highlight.js +++ b/browser/devtools/debugger/test/browser_dbg_on-pause-highlight.js @@ -30,13 +30,15 @@ function testPause() { gDebugger.gThreadClient.addOneTimeListener("paused", () => { gToolbox.selectTool("webconsole").then(() => { - ok(gToolboxTab.classList.contains("highlighted"), + ok(gToolboxTab.hasAttribute("highlighted") && + gToolboxTab.getAttribute("highlighted") == "true", "The highlighted class is present"); ok(!gToolboxTab.hasAttribute("selected") || gToolboxTab.getAttribute("selected") != "true", "The tab is not selected"); }).then(() => gToolbox.selectTool("jsdebugger")).then(() => { - ok(gToolboxTab.classList.contains("highlighted"), + ok(gToolboxTab.hasAttribute("highlighted") && + gToolboxTab.getAttribute("highlighted") == "true", "The highlighted class is present"); ok(gToolboxTab.hasAttribute("selected") && gToolboxTab.getAttribute("selected") == "true", diff --git a/browser/devtools/debugger/test/browser_dbg_on-pause-raise.js b/browser/devtools/debugger/test/browser_dbg_on-pause-raise.js index 24c354fe1f34..a911956f40cc 100644 --- a/browser/devtools/debugger/test/browser_dbg_on-pause-raise.js +++ b/browser/devtools/debugger/test/browser_dbg_on-pause-raise.js @@ -78,13 +78,15 @@ function testPause() { "Debugger's tab got selected."); } gToolbox.selectTool("webconsole").then(() => { - ok(gToolboxTab.classList.contains("highlighted"), + ok(gToolboxTab.hasAttribute("highlighted") && + gToolboxTab.getAttribute("highlighted") == "true", "The highlighted class is present"); ok(!gToolboxTab.hasAttribute("selected") || gToolboxTab.getAttribute("selected") != "true", "The tab is not selected"); }).then(() => gToolbox.selectTool("jsdebugger")).then(() => { - ok(gToolboxTab.classList.contains("highlighted"), + ok(gToolboxTab.hasAttribute("highlighted") && + gToolboxTab.getAttribute("highlighted") == "true", "The highlighted class is present"); ok(gToolboxTab.hasAttribute("selected") && gToolboxTab.getAttribute("selected") == "true", @@ -100,7 +102,8 @@ function testPause() { function testResume() { gDebugger.gThreadClient.addOneTimeListener("resumed", () => { gToolbox.selectTool("webconsole").then(() => { - ok(!gToolboxTab.classList.contains("highlighted"), + ok(!gToolboxTab.hasAttribute("highlighted") || + gToolboxTab.getAttribute("highlighted") != "true", "The highlighted class is not present now after the resume"); ok(!gToolboxTab.hasAttribute("selected") || gToolboxTab.getAttribute("selected") != "true", diff --git a/browser/devtools/framework/test/browser_toolbox_highlight.js b/browser/devtools/framework/test/browser_toolbox_highlight.js index 3ba74a4b742d..2da61edc290e 100644 --- a/browser/devtools/framework/test/browser_toolbox_highlight.js +++ b/browser/devtools/framework/test/browser_toolbox_highlight.js @@ -65,20 +65,20 @@ function unhighlightTab(toolId) { function checkHighlighted(toolId) { let tab = toolbox.doc.getElementById("toolbox-tab-" + toolId); - ok(tab.classList.contains("highlighted"), "The highlighted class is present"); + ok(tab.hasAttribute("highlighted"), "The highlighted attribute is present"); ok(!tab.hasAttribute("selected") || tab.getAttribute("selected") != "true", "The tab is not selected"); } function checkNoHighlightWhenSelected(toolId) { let tab = toolbox.doc.getElementById("toolbox-tab-" + toolId); - ok(tab.classList.contains("highlighted"), "The highlighted class is present"); + ok(tab.hasAttribute("highlighted"), "The highlighted attribute is present"); ok(tab.hasAttribute("selected") && tab.getAttribute("selected") == "true", "and the tab is selected, so the orange glow will not be present."); } function checkNoHighlight(toolId) { let tab = toolbox.doc.getElementById("toolbox-tab-" + toolId); - ok(!tab.classList.contains("highlighted"), - "The highlighted class is not present"); + ok(!tab.hasAttribute("highlighted"), + "The highlighted attribute is not present"); } diff --git a/browser/devtools/framework/toolbox.js b/browser/devtools/framework/toolbox.js index dcfb9c07a804..11f0bbe29bfc 100644 --- a/browser/devtools/framework/toolbox.js +++ b/browser/devtools/framework/toolbox.js @@ -796,7 +796,7 @@ Toolbox.prototype = { */ highlightTool: function(id) { let tab = this.doc.getElementById("toolbox-tab-" + id); - tab && tab.classList.add("highlighted"); + tab && tab.setAttribute("highlighted", "true"); }, /** @@ -807,7 +807,7 @@ Toolbox.prototype = { */ unhighlightTool: function(id) { let tab = this.doc.getElementById("toolbox-tab-" + id); - tab && tab.classList.remove("highlighted"); + tab && tab.removeAttribute("highlighted"); }, /** diff --git a/browser/themes/shared/devtools/toolbars.inc.css b/browser/themes/shared/devtools/toolbars.inc.css index 7e418b90c5f1..8b76d577bb91 100644 --- a/browser/themes/shared/devtools/toolbars.inc.css +++ b/browser/themes/shared/devtools/toolbars.inc.css @@ -51,7 +51,7 @@ display: none; } -.devtools-toolbarbutton:not([checked=true]):hover:active { +.devtools-toolbarbutton:not([checked]):hover:active { border-color: hsla(210,8%,5%,.6); background: linear-gradient(hsla(220,6%,10%,.3), hsla(212,7%,57%,.15) 65%, hsla(212,7%,57%,.3)); box-shadow: 0 0 3px hsla(210,8%,5%,.25) inset, 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 0 hsla(210,16%,76%,.15); @@ -282,28 +282,28 @@ background-image: linear-gradient(hsla(206,37%,4%,.4), hsla(206,37%,4%,.4)), @smallSeparator@; } -.devtools-sidebar-tabs > tabs > tab[selected=true] + tab { +.devtools-sidebar-tabs > tabs > tab[selected] + tab { background-image: linear-gradient(transparent, transparent), @solidSeparator@; } -.devtools-sidebar-tabs > tabs > tab[selected=true] + tab:hover { +.devtools-sidebar-tabs > tabs > tab[selected] + tab:hover { background-image: linear-gradient(hsla(206,37%,4%,.2), hsla(206,37%,4%,.2)), @solidSeparator@; } -.devtools-sidebar-tabs > tabs > tab[selected=true] + tab:hover:active { +.devtools-sidebar-tabs > tabs > tab[selected] + tab:hover:active { background-image: linear-gradient(hsla(206,37%,4%,.4), hsla(206,37%,4%,.4)), @solidSeparator@; } -.devtools-sidebar-tabs > tabs > tab[selected=true] { +.devtools-sidebar-tabs > tabs > tab[selected] { color: #f5f7fa; background-image: linear-gradient(#1d4f73, #1d4f73), @solidSeparator@; } -.devtools-sidebar-tabs > tabs > tab[selected=true]:hover { +.devtools-sidebar-tabs > tabs > tab[selected]:hover { background-image: linear-gradient(#274f64, #274f64), @solidSeparator@; } -.devtools-sidebar-tabs > tabs > tab[selected=true]:hover:active { +.devtools-sidebar-tabs > tabs > tab[selected]:hover:active { background-image: linear-gradient(#1f3e4f, #1f3e4f), @solidSeparator@; } @@ -520,7 +520,7 @@ } .devtools-tab:active > image, -.devtools-tab[selected=true] > image { +.devtools-tab[selected] > image { opacity: 1; } @@ -534,7 +534,7 @@ color: #f5f7fa; } -#toolbox-tabs .devtools-tab[selected=true] { +#toolbox-tabs .devtools-tab[selected] { color: #f5f7fa; background-color: #1a4666; box-shadow: 0 2px 0 #d7f1ff inset, @@ -542,32 +542,32 @@ 0 -2px 0 rgba(0,0,0,.2) inset; } -.devtools-tab[selected=true]:not(:first-child), -.devtools-tab.highlighted:not(:first-child) { +.devtools-tab[selected]:not(:first-child), +.devtools-tab[highlighted]:not(:first-child) { border-width: 0; -moz-padding-start: 1px; } -.devtools-tab[selected=true]:last-child, -.devtools-tab.highlighted:last-child { +.devtools-tab[selected]:last-child, +.devtools-tab[highlighted]:last-child { -moz-padding-end: 1px; } -.devtools-tab[selected=true] + .devtools-tab, -.devtools-tab.highlighted + .devtools-tab { +.devtools-tab[selected] + .devtools-tab, +.devtools-tab[highlighted] + .devtools-tab { -moz-border-start-width: 0; -moz-padding-start: 1px; } -.devtools-tab:not([selected=true]).highlighted { +.devtools-tab:not([selected])[highlighted] { color: #f5f7fa; background-color: hsla(99,100%,14%,.2); box-shadow: 0 2px 0 #7bc107 inset; } -.devtools-tab:not(.highlighted) > .highlighted-icon, -.devtools-tab[selected=true] > .highlighted-icon, -.devtools-tab:not([selected=true]).highlighted > .default-icon { +.devtools-tab:not([highlighted]) > .highlighted-icon, +.devtools-tab[selected] > .highlighted-icon, +.devtools-tab:not([selected])[highlighted] > .default-icon { visibility: collapse; } From 807526aae40e727c9d6e73b0d70edd3c785d5e9a Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Fri, 20 Dec 2013 19:05:18 +0200 Subject: [PATCH 06/18] Bug 952421 - Disabling and re-enabling a breakpoint turns it into a conditional breakpoint with an undefined expression, r=past --- .../devtools/debugger/debugger-controller.js | 5 +- browser/devtools/debugger/test/browser.ini | 1 + .../browser_dbg_conditional-breakpoints-04.js | 84 +++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-04.js diff --git a/browser/devtools/debugger/debugger-controller.js b/browser/devtools/debugger/debugger-controller.js index f03d25c453a7..74acab2c7604 100644 --- a/browser/devtools/debugger/debugger-controller.js +++ b/browser/devtools/debugger/debugger-controller.js @@ -1928,7 +1928,10 @@ Breakpoints.prototype = { let disabledPromise = this._disabled.get(identifier); if (disabledPromise) { disabledPromise.then(({ conditionalExpression: previousValue }) => { - aBreakpointClient.conditionalExpression = previousValue; + // Setting a falsy conditional expression is redundant. + if (previousValue) { + aBreakpointClient.conditionalExpression = previousValue; + } }); this._disabled.delete(identifier); } diff --git a/browser/devtools/debugger/test/browser.ini b/browser/devtools/debugger/test/browser.ini index 373ff46ac700..000fa52e2b8a 100644 --- a/browser/devtools/debugger/test/browser.ini +++ b/browser/devtools/debugger/test/browser.ini @@ -100,6 +100,7 @@ support-files = [browser_dbg_conditional-breakpoints-01.js] [browser_dbg_conditional-breakpoints-02.js] [browser_dbg_conditional-breakpoints-03.js] +[browser_dbg_conditional-breakpoints-04.js] [browser_dbg_controller-evaluate-01.js] [browser_dbg_controller-evaluate-02.js] [browser_dbg_debugger-statement.js] diff --git a/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-04.js b/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-04.js new file mode 100644 index 000000000000..055a0864b120 --- /dev/null +++ b/browser/devtools/debugger/test/browser_dbg_conditional-breakpoints-04.js @@ -0,0 +1,84 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Make sure that conditional breakpoints with undefined expressions + * are stored as plain breakpoints when re-enabling them. + */ + +const TAB_URL = EXAMPLE_URL + "doc_conditional-breakpoints.html"; + +function test() { + let gTab, gDebuggee, gPanel, gDebugger; + let gSources, gBreakpoints, gLocation; + + initDebugger(TAB_URL).then(([aTab, aDebuggee, aPanel]) => { + gTab = aTab; + gDebuggee = aDebuggee; + gPanel = aPanel; + gDebugger = gPanel.panelWin; + gSources = gDebugger.DebuggerView.Sources; + gBreakpoints = gDebugger.DebuggerController.Breakpoints; + + gLocation = { url: gSources.selectedValue, line: 18 }; + + waitForSourceAndCaretAndScopes(gPanel, ".html", 17) + .then(addBreakpoint) + .then(setDummyConditional) + .then(() => { + let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.BREAKPOINT_REMOVED); + toggleBreakpoint(); + return finished; + }) + .then(() => { + let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.BREAKPOINT_ADDED); + toggleBreakpoint(); + return finished; + }) + .then(testConditionalExpressionOnClient) + .then(() => { + let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.CONDITIONAL_BREAKPOINT_POPUP_SHOWING); + openConditionalPopup(); + finished.then(() => ok(false, "The popup shouldn't have opened.")); + return waitForTime(1000); + }) + .then(() => resumeDebuggerThenCloseAndFinish(gPanel)) + .then(null, aError => { + ok(false, "Got an error: " + aError.message + "\n" + aError.stack); + }); + + gDebuggee.ermahgerd(); + }); + + function addBreakpoint() { + return gPanel.addBreakpoint(gLocation); + } + + function setDummyConditional(aClient) { + // This happens when a conditional expression input popup is shown + // but the user doesn't type anything into it. + aClient.conditionalExpression = ""; + } + + function toggleBreakpoint() { + EventUtils.sendMouseEvent({ type: "click" }, + gDebugger.document.querySelector(".dbg-breakpoint-checkbox"), + gDebugger); + } + + function openConditionalPopup() { + EventUtils.sendMouseEvent({ type: "click" }, + gDebugger.document.querySelector(".dbg-breakpoint"), + gDebugger); + } + + function testConditionalExpressionOnClient() { + return gBreakpoints._getAdded(gLocation).then(aClient => { + if ("conditionalExpression" in aClient) { + ok(false, "A conditional expression shouldn't have been set."); + } else { + ok(true, "The conditional expression wasn't set, as expected."); + } + }); + } +} From e8dea1b10ae7965acbbcdb1385c434cbac0a0c20 Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Thu, 19 Dec 2013 15:04:59 +0200 Subject: [PATCH 07/18] Bug 843004 - Part 2: ObjectActor grip changes to allow pretty output; r=benvie,past --- toolkit/devtools/DevToolsUtils.js | 22 + toolkit/devtools/DevToolsUtils.jsm | 1 + toolkit/devtools/server/actors/script.js | 656 ++++++++++++++++++- toolkit/devtools/server/actors/webconsole.js | 8 + 4 files changed, 664 insertions(+), 23 deletions(-) diff --git a/toolkit/devtools/DevToolsUtils.js b/toolkit/devtools/DevToolsUtils.js index 570ed752f6f6..9d88e3de2080 100644 --- a/toolkit/devtools/DevToolsUtils.js +++ b/toolkit/devtools/DevToolsUtils.js @@ -6,6 +6,7 @@ /* General utilities used throughout devtools. */ +let Cu = Components.utils; let { Promise: promise } = Components.utils.import("resource://gre/modules/commonjs/sdk/core/promise.js", {}); let { Services } = Components.utils.import("resource://gre/modules/Services.jsm", {}); @@ -237,3 +238,24 @@ this.hasSafeGetter = function hasSafeGetter(aDesc) { return fn && fn.callable && fn.class == "Function" && fn.script === undefined; }; +/** + * Check if it is safe to read properties and execute methods from the given JS + * object. Safety is defined as being protected from unintended code execution + * from content scripts (or cross-compartment code). + * + * See bugs 945920 and 946752 for discussion. + * + * @type Object aObj + * The object to check. + * @return Boolean + * True if it is safe to read properties from aObj, or false otherwise. + */ +this.isSafeJSObject = function isSafeJSObject(aObj) { + if (Cu.getGlobalForObject(aObj) == + Cu.getGlobalForObject(isSafeJSObject)) { + return true; // aObj is not a cross-compartment wrapper. + } + + return Cu.isXrayWrapper(aObj); +}; + diff --git a/toolkit/devtools/DevToolsUtils.jsm b/toolkit/devtools/DevToolsUtils.jsm index a505e1cdc7c5..a9bac1bedebe 100644 --- a/toolkit/devtools/DevToolsUtils.jsm +++ b/toolkit/devtools/DevToolsUtils.jsm @@ -28,4 +28,5 @@ this.DevToolsUtils = { defineLazyPrototypeGetter: defineLazyPrototypeGetter, getProperty: getProperty, hasSafeGetter: hasSafeGetter, + isSafeJSObject: isSafeJSObject, }; diff --git a/toolkit/devtools/server/actors/script.js b/toolkit/devtools/server/actors/script.js index 670b8d209909..b2b7bade3cbb 100644 --- a/toolkit/devtools/server/actors/script.js +++ b/toolkit/devtools/server/actors/script.js @@ -6,6 +6,14 @@ "use strict"; +let TYPED_ARRAY_CLASSES = ["Uint8Array", "Uint8ClampedArray", "Uint16Array", + "Uint32Array", "Int8Array", "Int16Array", "Int32Array", "Float32Array", + "Float64Array"]; + +// Number of items to preview in objects, arrays, maps, sets, lists, +// collections, etc. +let OBJECT_PREVIEW_MAX_ITEMS = 10; + /** * BreakpointStore objects keep track of all breakpoints that get set so that we * can reset them when the same script is introduced to the thread again (such @@ -440,6 +448,8 @@ function ThreadActor(aHooks, aGlobal) this._options = { useSourceMaps: false }; + + this._gripDepth = 0; } /** @@ -449,6 +459,9 @@ function ThreadActor(aHooks, aGlobal) ThreadActor.breakpointStore = new BreakpointStore(); ThreadActor.prototype = { + // Used by the ObjectActor to keep track of the depth of grip() calls. + _gripDepth: null, + actorPrefix: "context", get state() { return this._state; }, @@ -2730,7 +2743,7 @@ function errorStringify(aObj) { * The stringification for the object. */ function stringify(aObj) { - if (Cu.isDeadWrapper(aObj)) { + if (aObj.class == "DeadObject") { const error = new Error("Dead object encountered."); DevToolsUtils.reportException("stringify", error); return ""; @@ -2829,6 +2842,8 @@ ObjectActor.prototype = { * Returns a grip for this actor for returning in a protocol message. */ grip: function () { + this.threadActor._gripDepth++; + let g = { "type": "object", "class": this.obj.class, @@ -2838,29 +2853,22 @@ ObjectActor.prototype = { "sealed": this.obj.isSealed() }; - // Add additional properties for functions. - if (this.obj.class === "Function") { - if (this.obj.name) { - g.name = this.obj.name; - } - if (this.obj.displayName) { - g.displayName = this.obj.displayName; + if (this.obj.class != "DeadObject") { + let raw = Cu.unwaiveXrays(this.obj.unsafeDereference()); + if (!DevToolsUtils.isSafeJSObject(raw)) { + raw = null; } - // Check if the developer has added a de-facto standard displayName - // property for us to use. - try { - let desc = this.obj.getOwnPropertyDescriptor("displayName"); - if (desc && desc.value && typeof desc.value == "string") { - g.userDisplayName = this.threadActor.createValueGrip(desc.value); + let previewers = DebuggerServer.ObjectActorPreviewers[this.obj.class] || + DebuggerServer.ObjectActorPreviewers.Object; + for (let fn of previewers) { + if (fn(this, g, raw)) { + break; } - } catch (e) { - // Calling getOwnPropertyDescriptor with displayName might throw - // with "permission denied" errors for some functions. - dumpn(e); } } + this.threadActor._gripDepth--; return g; }, @@ -2964,15 +2972,17 @@ ObjectActor.prototype = { * @param object aOwnProperties * The object that holds the list of known ownProperties for * |this.obj|. + * @param number [aLimit=0] + * Optional limit of getter values to find. * @return object * An object that maps property names to safe getter descriptors as * defined by the remote debugging protocol. */ - _findSafeGetterValues: function (aOwnProperties) + _findSafeGetterValues: function (aOwnProperties, aLimit = 0) { let safeGetterValues = Object.create(null); let obj = this.obj; - let level = 0; + let level = 0, i = 0; while (obj) { let getters = this._findSafeGetters(obj); @@ -3014,9 +3024,15 @@ ObjectActor.prototype = { enumerable: desc.enumerable, writable: level == 0 ? desc.writable : true, }; + if (aLimit && ++i == aLimit) { + break; + } } } } + if (aLimit && i == aLimit) { + break; + } obj = obj.proto; level++; @@ -3043,7 +3059,15 @@ ObjectActor.prototype = { } let getters = new Set(); - for (let name of aObject.getOwnPropertyNames()) { + let names = []; + try { + names = aObject.getOwnPropertyNames() + } catch (ex) { + // Calling getOwnPropertyNames() on some wrapped native prototypes is not + // allowed: "cannot modify properties of a WrappedNative". See bug 952093. + } + + for (let name of names) { let desc = null; try { desc = aObject.getOwnPropertyDescriptor(name); @@ -3108,10 +3132,17 @@ ObjectActor.prototype = { * A helper method that creates a property descriptor for the provided object, * properly formatted for sending in a protocol response. * + * @private * @param string aName * The property that the descriptor is generated for. + * @param boolean [aOnlyEnumerable] + * Optional: true if you want a descriptor only for an enumerable + * property, false otherwise. + * @return object|undefined + * The property descriptor, or undefined if this is not an enumerable + * property and aOnlyEnumerable=true. */ - _propertyDescriptor: function (aName) { + _propertyDescriptor: function (aName, aOnlyEnumerable) { let desc; try { desc = this.obj.getOwnPropertyDescriptor(aName); @@ -3127,7 +3158,7 @@ ObjectActor.prototype = { }; } - if (!desc) { + if (!desc || aOnlyEnumerable && !desc.enumerable) { return undefined; } @@ -3232,6 +3263,565 @@ ObjectActor.prototype.requestTypes = { }; +/** + * Functions for adding information to ObjectActor grips for the purpose of + * having customized output. This object holds arrays mapped by + * Debugger.Object.prototype.class. + * + * In each array you can add functions that take two + * arguments: + * - the ObjectActor instance to make a preview for, + * - the grip object being prepared for the client, + * - the raw JS object after calling Debugger.Object.unsafeDereference(). This + * argument is only provided if the object is safe for reading properties and + * executing methods. See DevToolsUtils.isSafeJSObject(). + * + * Functions must return false if they cannot provide preview + * information for the debugger object, or true otherwise. + */ +DebuggerServer.ObjectActorPreviewers = { + Function: [function({obj, threadActor}, aGrip) { + if (obj.name) { + aGrip.name = obj.name; + } + + if (obj.displayName) { + aGrip.displayName = obj.displayName.substr(0, 500); + } + + if (obj.parameterNames) { + aGrip.parameterNames = obj.parameterNames; + } + + // Check if the developer has added a de-facto standard displayName + // property for us to use. + let userDisplayName; + try { + userDisplayName = obj.getOwnPropertyDescriptor("displayName"); + } catch (e) { + // Calling getOwnPropertyDescriptor with displayName might throw + // with "permission denied" errors for some functions. + dumpn(e); + } + + if (userDisplayName && typeof userDisplayName.value == "string" && + userDisplayName.value) { + aGrip.userDisplayName = threadActor.createValueGrip(userDisplayName.value); + } + + return true; + }], + + RegExp: [function({obj, threadActor}, aGrip) { + // Avoid having any special preview for the RegExp.prototype itself. + if (!obj.proto || obj.proto.class != "RegExp") { + return false; + } + + let str = RegExp.prototype.toString.call(obj.unsafeDereference()); + aGrip.displayString = threadActor.createValueGrip(str); + return true; + }], + + Date: [function({obj, threadActor}, aGrip) { + if (!obj.proto || obj.proto.class != "Date") { + return false; + } + + let time = Date.prototype.getTime.call(obj.unsafeDereference()); + + aGrip.preview = { + timestamp: threadActor.createValueGrip(time), + }; + return true; + }], + + Array: [function({obj, threadActor}, aGrip) { + let length = DevToolsUtils.getProperty(obj, "length"); + if (typeof length != "number") { + return false; + } + + aGrip.preview = { + kind: "ArrayLike", + length: length, + }; + + if (threadActor._gripDepth > 1) { + return true; + } + + let raw = obj.unsafeDereference(); + let items = aGrip.preview.items = []; + + for (let [i, value] of Array.prototype.entries.call(raw)) { + if (Object.hasOwnProperty.call(raw, i)) { + value = makeDebuggeeValueIfNeeded(obj, value); + items.push(threadActor.createValueGrip(value)); + } else { + items.push(null); + } + + if (items.length == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + + return true; + }], // Array + + Set: [function({obj, threadActor}, aGrip) { + let size = DevToolsUtils.getProperty(obj, "size"); + if (typeof size != "number") { + return false; + } + + aGrip.preview = { + kind: "ArrayLike", + length: size, + }; + + // Avoid recursive object grips. + if (threadActor._gripDepth > 1) { + return true; + } + + let raw = obj.unsafeDereference(); + let items = aGrip.preview.items = []; + for (let item of Set.prototype.values.call(raw)) { + item = makeDebuggeeValueIfNeeded(obj, item); + items.push(threadActor.createValueGrip(item)); + if (items.length == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + + return true; + }], // Set + + Map: [function({obj, threadActor}, aGrip) { + let size = DevToolsUtils.getProperty(obj, "size"); + if (typeof size != "number") { + return false; + } + + aGrip.preview = { + kind: "MapLike", + size: size, + }; + + if (threadActor._gripDepth > 1) { + return true; + } + + let raw = obj.unsafeDereference(); + let entries = aGrip.preview.entries = []; + for (let [key, value] of Map.prototype.entries.call(raw)) { + key = makeDebuggeeValueIfNeeded(obj, key); + value = makeDebuggeeValueIfNeeded(obj, value); + entries.push([threadActor.createValueGrip(key), + threadActor.createValueGrip(value)]); + if (entries.length == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + + return true; + }], // Map + + DOMStringMap: [function({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj) { + return false; + } + + let keys = obj.getOwnPropertyNames(); + aGrip.preview = { + kind: "MapLike", + size: keys.length, + }; + + if (threadActor._gripDepth > 1) { + return true; + } + + let entries = aGrip.preview.entries = []; + for (let key of keys) { + let value = makeDebuggeeValueIfNeeded(obj, aRawObj[key]); + entries.push([key, threadActor.createValueGrip(value)]); + if (entries.length == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + + return true; + }], // DOMStringMap +}; // DebuggerServer.ObjectActorPreviewers + +// Preview functions that do not rely on the object class. +DebuggerServer.ObjectActorPreviewers.Object = [ + function TypedArray({obj, threadActor}, aGrip) { + if (TYPED_ARRAY_CLASSES.indexOf(obj.class) == -1) { + return false; + } + + let length = DevToolsUtils.getProperty(obj, "length"); + if (typeof length != "number") { + return false; + } + + aGrip.preview = { + kind: "ArrayLike", + length: length, + }; + + if (threadActor._gripDepth > 1) { + return true; + } + + let raw = obj.unsafeDereference(); + let global = Cu.getGlobalForObject(DebuggerServer); + let classProto = global[obj.class].prototype; + let safeView = classProto.subarray.call(raw, 0, OBJECT_PREVIEW_MAX_ITEMS); + let items = aGrip.preview.items = []; + for (let i = 0; i < safeView.length; i++) { + items.push(safeView[i]); + } + + return true; + }, + + function Error({obj, threadActor}, aGrip) { + switch (obj.class) { + case "Error": + case "EvalError": + case "RangeError": + case "ReferenceError": + case "SyntaxError": + case "TypeError": + case "URIError": + let name = DevToolsUtils.getProperty(obj, "name"); + let msg = DevToolsUtils.getProperty(obj, "message"); + let stack = DevToolsUtils.getProperty(obj, "stack"); + let fileName = DevToolsUtils.getProperty(obj, "fileName"); + let lineNumber = DevToolsUtils.getProperty(obj, "lineNumber"); + let columnNumber = DevToolsUtils.getProperty(obj, "columnNumber"); + aGrip.preview = { + kind: "Error", + name: threadActor.createValueGrip(name), + message: threadActor.createValueGrip(msg), + stack: threadActor.createValueGrip(stack), + fileName: threadActor.createValueGrip(fileName), + lineNumber: threadActor.createValueGrip(lineNumber), + columnNumber: threadActor.createValueGrip(columnNumber), + }; + return true; + default: + return false; + } + }, + + function CSSMediaRule({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || !(aRawObj instanceof Ci.nsIDOMCSSMediaRule)) { + return false; + } + aGrip.preview = { + kind: "ObjectWithText", + text: threadActor.createValueGrip(aRawObj.conditionText), + }; + return true; + }, + + function CSSStyleRule({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || !(aRawObj instanceof Ci.nsIDOMCSSStyleRule)) { + return false; + } + aGrip.preview = { + kind: "ObjectWithText", + text: threadActor.createValueGrip(aRawObj.selectorText), + }; + return true; + }, + + function ObjectWithURL({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || + !(aRawObj instanceof Ci.nsIDOMCSSImportRule || + aRawObj instanceof Ci.nsIDOMCSSStyleSheet || + aRawObj instanceof Ci.nsIDOMLocation || + aRawObj instanceof Ci.nsIDOMWindow)) { + return false; + } + + let url; + if (aRawObj instanceof Ci.nsIDOMWindow) { + url = aRawObj.location.href; + } else { + url = aRawObj.href; + } + + aGrip.preview = { + kind: "ObjectWithURL", + url: threadActor.createValueGrip(url), + }; + + return true; + }, + + function ArrayLike({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || + obj.class != "DOMTokenList" && + !(aRawObj instanceof Ci.nsIDOMMozNamedAttrMap || + aRawObj instanceof Ci.nsIDOMCSSRuleList || + aRawObj instanceof Ci.nsIDOMCSSValueList || + aRawObj instanceof Ci.nsIDOMDOMStringList || + aRawObj instanceof Ci.nsIDOMFileList || + aRawObj instanceof Ci.nsIDOMFontFaceList || + aRawObj instanceof Ci.nsIDOMMediaList || + aRawObj instanceof Ci.nsIDOMNodeList || + aRawObj instanceof Ci.nsIDOMStyleSheetList)) { + return false; + } + + if (typeof aRawObj.length != "number") { + return false; + } + + aGrip.preview = { + kind: "ArrayLike", + length: aRawObj.length, + }; + + if (threadActor._gripDepth > 1) { + return true; + } + + let items = aGrip.preview.items = []; + + for (let i = 0; i < aRawObj.length && + items.length < OBJECT_PREVIEW_MAX_ITEMS; i++) { + let value = makeDebuggeeValueIfNeeded(obj, aRawObj[i]); + items.push(threadActor.createValueGrip(value)); + } + + return true; + }, // ArrayLike + + function CSSStyleDeclaration({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || !(aRawObj instanceof Ci.nsIDOMCSSStyleDeclaration)) { + return false; + } + + aGrip.preview = { + kind: "MapLike", + size: aRawObj.length, + }; + + let entries = aGrip.preview.entries = []; + + for (let i = 0; i < OBJECT_PREVIEW_MAX_ITEMS && + i < aRawObj.length; i++) { + let prop = aRawObj[i]; + let value = aRawObj.getPropertyValue(prop); + entries.push([prop, threadActor.createValueGrip(value)]); + } + + return true; + }, + + function DOMNode({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || !(aRawObj instanceof Ci.nsIDOMNode)) { + return false; + } + + let preview = aGrip.preview = { + kind: "DOMNode", + nodeType: aRawObj.nodeType, + nodeName: aRawObj.nodeName, + }; + + if (aRawObj instanceof Ci.nsIDOMDocument) { + preview.location = threadActor.createValueGrip(aRawObj.location.href); + } else if (aRawObj instanceof Ci.nsIDOMDocumentFragment) { + preview.childNodesLength = aRawObj.childNodes.length; + + if (threadActor._gripDepth < 2) { + preview.childNodes = []; + for (let node of aRawObj.childNodes) { + let actor = threadActor.createValueGrip(obj.makeDebuggeeValue(node)); + preview.childNodes.push(actor); + if (preview.childNodes.length == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + } + } else if (aRawObj instanceof Ci.nsIDOMElement) { + // Add preview for DOM element attributes. + if (aRawObj instanceof Ci.nsIDOMHTMLElement) { + preview.nodeName = preview.nodeName.toLowerCase(); + } + + let i = 0; + preview.attributes = {}; + preview.attributesLength = aRawObj.attributes.length; + for (let attr of aRawObj.attributes) { + preview.attributes[attr.nodeName] = threadActor.createValueGrip(attr.value); + if (++i == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + } else if (aRawObj instanceof Ci.nsIDOMAttr) { + preview.value = threadActor.createValueGrip(aRawObj.value); + } else if (aRawObj instanceof Ci.nsIDOMText || + aRawObj instanceof Ci.nsIDOMComment) { + preview.textContent = threadActor.createValueGrip(aRawObj.textContent); + } + + return true; + }, // DOMNode + + function DOMEvent({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || !(aRawObj instanceof Ci.nsIDOMEvent)) { + return false; + } + + let preview = aGrip.preview = { + kind: "DOMEvent", + type: aRawObj.type, + properties: Object.create(null), + }; + + if (threadActor._gripDepth < 2) { + let target = obj.makeDebuggeeValue(aRawObj.target); + preview.target = threadActor.createValueGrip(target); + } + + let props = []; + if (aRawObj instanceof Ci.nsIDOMMouseEvent) { + props.push("buttons", "clientX", "clientY", "layerX", "layerY"); + } else if (aRawObj instanceof Ci.nsIDOMKeyEvent) { + let modifiers = []; + if (aRawObj.altKey) { + modifiers.push("Alt"); + } + if (aRawObj.ctrlKey) { + modifiers.push("Control"); + } + if (aRawObj.metaKey) { + modifiers.push("Meta"); + } + if (aRawObj.shiftKey) { + modifiers.push("Shift"); + } + preview.eventKind = "key"; + preview.modifiers = modifiers; + + props.push("key", "charCode", "keyCode"); + } else if (aRawObj instanceof Ci.nsIDOMTransitionEvent || + aRawObj instanceof Ci.nsIDOMAnimationEvent) { + props.push("animationName", "pseudoElement"); + } else if (aRawObj instanceof Ci.nsIDOMClipboardEvent) { + props.push("clipboardData"); + } + + // Add event-specific properties. + for (let prop of props) { + let value = aRawObj[prop]; + if (value && (typeof value == "object" || typeof value == "function")) { + // Skip properties pointing to objects. + if (threadActor._gripDepth > 1) { + continue; + } + value = obj.makeDebuggeeValue(value); + } + preview.properties[prop] = threadActor.createValueGrip(value); + } + + // Add any properties we find on the event object. + if (!props.length) { + let i = 0; + for (let prop in aRawObj) { + let value = aRawObj[prop]; + if (prop == "target" || prop == "type" || value === null || + typeof value == "function") { + continue; + } + if (value && typeof value == "object") { + if (threadActor._gripDepth > 1) { + continue; + } + value = obj.makeDebuggeeValue(value); + } + preview.properties[prop] = threadActor.createValueGrip(value); + if (++i == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + } + + return true; + }, // DOMEvent + + function DOMException({obj, threadActor}, aGrip, aRawObj) { + if (!aRawObj || !(aRawObj instanceof Ci.nsIDOMDOMException)) { + return false; + } + + aGrip.preview = { + kind: "DOMException", + name: threadActor.createValueGrip(aRawObj.name), + message: threadActor.createValueGrip(aRawObj.message), + code: threadActor.createValueGrip(aRawObj.code), + result: threadActor.createValueGrip(aRawObj.result), + filename: threadActor.createValueGrip(aRawObj.filename), + lineNumber: threadActor.createValueGrip(aRawObj.lineNumber), + columnNumber: threadActor.createValueGrip(aRawObj.columnNumber), + }; + + return true; + }, + + function GenericObject(aObjectActor, aGrip) { + let {obj, threadActor} = aObjectActor; + if (aGrip.preview || aGrip.displayString || threadActor._gripDepth > 1) { + return false; + } + + let i = 0, names = []; + let preview = aGrip.preview = { + kind: "Object", + ownProperties: Object.create(null), + }; + + try { + names = obj.getOwnPropertyNames(); + } catch (ex) { + // Calling getOwnPropertyNames() on some wrapped native prototypes is not + // allowed: "cannot modify properties of a WrappedNative". See bug 952093. + } + + preview.ownPropertiesLength = names.length; + + for (let name of names) { + let desc = aObjectActor._propertyDescriptor(name, true); + if (!desc) { + continue; + } + + preview.ownProperties[name] = desc; + if (++i == OBJECT_PREVIEW_MAX_ITEMS) { + break; + } + } + + if (i < OBJECT_PREVIEW_MAX_ITEMS) { + preview.safeGetterValues = aObjectActor. + _findSafeGetterValues(preview.ownProperties, + OBJECT_PREVIEW_MAX_ITEMS - i); + } + + return true; + }, // GenericObject +]; // DebuggerServer.ObjectActorPreviewers.Object + /** * Creates a pause-scoped actor for the specified object. * @see ObjectActor @@ -4517,3 +5107,23 @@ function positionInNodeList(element, nodeList) { } return -1; } + +/** + * Make a debuggee value for the given object, if needed. Primitive values + * are left the same. + * + * Use case: you have a raw JS object (after unsafe dereference) and you want to + * send it to the client. In that case you need to use an ObjectActor which + * requires a debuggee value. The Debugger.Object.prototype.makeDebuggeeValue() + * method works only for JS objects and functions. + * + * @param Debugger.Object obj + * @param any value + * @return object + */ +function makeDebuggeeValueIfNeeded(obj, value) { + if (value && (typeof value == "object" || typeof value == "function")) { + return obj.makeDebuggeeValue(value); + } + return value; +} diff --git a/toolkit/devtools/server/actors/webconsole.js b/toolkit/devtools/server/actors/webconsole.js index 00973ef12fe7..cfbe977cf833 100644 --- a/toolkit/devtools/server/actors/webconsole.js +++ b/toolkit/devtools/server/actors/webconsole.js @@ -61,6 +61,7 @@ function WebConsoleActor(aConnection, aParentActor) this._protoChains = new Map(); this._netEvents = new Map(); + this._gripDepth = 0; this._onObserverNotification = this._onObserverNotification.bind(this); if (this.parentActor.isRootActor) { @@ -80,6 +81,13 @@ WebConsoleActor.prototype = */ dbg: null, + /** + * This is used by the ObjectActor to keep track of the depth of grip() calls. + * @private + * @type number + */ + _gripDepth: null, + /** * Actor pool for all of the actors we send to the client. * @private From e6218da369bd59148fbcba887a8a3cc22710b753 Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Wed, 18 Dec 2013 20:17:05 +0200 Subject: [PATCH 08/18] Bug 843004 - Part 3: VariablesView ObjectActor pretty output; r=benvie,vporof --- .../devtools/shared/widgets/VariablesView.jsm | 419 +++++++++++++++++- browser/devtools/webconsole/console-output.js | 19 +- browser/devtools/webconsole/webconsole.js | 6 +- .../browser/devtools/debugger.properties | 17 +- toolkit/devtools/webconsole/utils.js | 20 +- 5 files changed, 449 insertions(+), 32 deletions(-) diff --git a/browser/devtools/shared/widgets/VariablesView.jsm b/browser/devtools/shared/widgets/VariablesView.jsm index 9ab1bcc9aa46..5c29fbc40fe8 100644 --- a/browser/devtools/shared/widgets/VariablesView.jsm +++ b/browser/devtools/shared/widgets/VariablesView.jsm @@ -27,6 +27,9 @@ Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "devtools", "resource://gre/modules/devtools/Loader.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "PluralForm", + "resource://gre/modules/PluralForm.jsm"); + XPCOMUtils.defineLazyServiceGetter(this, "clipboardHelper", "@mozilla.org/widget/clipboardhelper;1", "nsIClipboardHelper"); @@ -2346,7 +2349,10 @@ Variable.prototype = Heritage.extend(Scope.prototype, { this._valueLabel.classList.remove(VariablesView.getClass(prevGrip)); } this._valueGrip = aGrip; - this._valueString = VariablesView.getString(aGrip, true); + this._valueString = VariablesView.getString(aGrip, { + concise: true, + noEllipsis: true, + }); this._valueClassName = VariablesView.getClass(aGrip); this._valueLabel.classList.add(this._valueClassName); @@ -3118,12 +3124,16 @@ VariablesView.getGrip = function(aValue) { * * @param any aGrip * @see Variable.setGrip - * @param boolean aConciseFlag - * Return a concisely formatted property string. + * @param object aOptions + * Options: + * - concise: boolean that tells you want a concisely formatted string. + * - noStringQuotes: boolean that tells to not quote strings. + * - noEllipsis: boolean that tells to not add an ellipsis after the + * initial text of a longString. * @return string * The formatted property string. */ -VariablesView.getString = function(aGrip, aConciseFlag) { +VariablesView.getString = function(aGrip, aOptions = {}) { if (aGrip && typeof aGrip == "object") { switch (aGrip.type) { case "undefined": @@ -3133,18 +3143,30 @@ VariablesView.getString = function(aGrip, aConciseFlag) { case "-Infinity": case "-0": return aGrip.type; - case "longString": - return "\"" + aGrip.initial + "\""; default: - if (!aConciseFlag) { - return "[" + aGrip.type + " " + aGrip.class + "]"; + let stringifier = VariablesView.stringifiers.byType[aGrip.type]; + if (stringifier) { + let result = stringifier(aGrip, aOptions); + if (result != null) { + return result; + } } - return aGrip.class; + + if (aGrip.displayString) { + return VariablesView.getString(aGrip.displayString, aOptions); + } + + if (aGrip.type == "object" && aOptions.concise) { + return aGrip.class; + } + + return "[" + aGrip.type + " " + aGrip.class + "]"; } } + switch (typeof aGrip) { case "string": - return "\"" + aGrip + "\""; + return VariablesView.stringifiers.byType.string(aGrip, aOptions); case "boolean": return aGrip ? "true" : "false"; case "number": @@ -3156,6 +3178,367 @@ VariablesView.getString = function(aGrip, aConciseFlag) { } }; +/** + * The VariablesView stringifiers are used by VariablesView.getString(). These + * are organized by object type, object class and by object actor preview kind. + * Some objects share identical ways for previews, for example Arrays, Sets and + * NodeLists. + * + * Any stringifier function must return a string. If null is returned, * then + * the default stringifier will be used. When invoked, the stringifier is + * given the same two arguments as those given to VariablesView.getString(). + */ +VariablesView.stringifiers = {}; + +VariablesView.stringifiers.byType = { + string: function(aGrip, {noStringQuotes}) { + if (noStringQuotes) { + return aGrip; + } + return uneval(aGrip); + }, + + longString: function({initial}, {noStringQuotes, noEllipsis}) { + let ellipsis = noEllipsis ? "" : Scope.ellipsis; + if (noStringQuotes) { + return initial + ellipsis; + } + let result = uneval(initial); + if (!ellipsis) { + return result; + } + return result.substr(0, result.length - 1) + ellipsis + '"'; + }, + + object: function(aGrip, aOptions) { + let {preview} = aGrip; + let stringifier; + if (preview && preview.kind) { + stringifier = VariablesView.stringifiers.byObjectKind[preview.kind]; + } + if (!stringifier && aGrip.class) { + stringifier = VariablesView.stringifiers.byObjectClass[aGrip.class]; + } + if (stringifier) { + return stringifier(aGrip, aOptions); + } + return null; + }, +}; // VariablesView.stringifiers.byType + +VariablesView.stringifiers.byObjectClass = { + Function: function(aGrip, {concise}) { + // TODO: Bug 948484 - support arrow functions and ES6 generators + + let name = aGrip.userDisplayName || aGrip.displayName || aGrip.name || ""; + name = VariablesView.getString(name, { noStringQuotes: true }); + + // TODO: Bug 948489 - Support functions with destructured parameters and + // rest parameters + let params = aGrip.parameterNames || ""; + if (!concise) { + return "function " + name + "(" + params + ")"; + } + return (name || "function ") + "(" + params + ")"; + }, + + RegExp: function({displayString}) { + return VariablesView.getString(displayString, { noStringQuotes: true }); + }, + + Date: function({preview}) { + if (!preview || !("timestamp" in preview)) { + return null; + } + + if (typeof preview.timestamp != "number") { + return new Date(preview.timestamp).toString(); // invalid date + } + + return "Date " + new Date(preview.timestamp).toISOString(); + }, +}; // VariablesView.stringifiers.byObjectClass + +VariablesView.stringifiers.byObjectKind = { + ArrayLike: function(aGrip, {concise}) { + let {preview} = aGrip; + if (concise) { + return aGrip.class + "[" + preview.length + "]"; + } + + if (!preview.items) { + return null; + } + + let shown = 0, result = [], lastHole = null; + for (let item of preview.items) { + if (item === null) { + if (lastHole !== null) { + result[lastHole] += ","; + } else { + result.push(""); + } + lastHole = result.length - 1; + } else { + lastHole = null; + result.push(VariablesView.getString(item, { concise: true })); + } + shown++; + } + + if (shown < preview.length) { + let n = preview.length - shown; + result.push(VariablesView.stringifiers._getNMoreString(n)); + } else if (lastHole !== null) { + // make sure we have the right number of commas... + result[lastHole] += ","; + } + + let prefix = aGrip.class == "Array" ? "" : aGrip.class + " "; + return prefix + "[" + result.join(", ") + "]"; + }, + + MapLike: function(aGrip, {concise}) { + let {preview} = aGrip; + if (concise || !preview.entries) { + let size = typeof preview.size == "number" ? + "[" + preview.size + "]" : ""; + return aGrip.class + size; + } + + let entries = []; + for (let [key, value] of preview.entries) { + let keyString = VariablesView.getString(key, { + concise: true, + noStringQuotes: true, + }); + let valueString = VariablesView.getString(value, { concise: true }); + entries.push(keyString + ": " + valueString); + } + + if (typeof preview.size == "number" && preview.size > entries.length) { + let n = preview.size - entries.length; + entries.push(VariablesView.stringifiers._getNMoreString(n)); + } + + return aGrip.class + " {" + entries.join(", ") + "}"; + }, + + ObjectWithText: function(aGrip, {concise}) { + if (concise) { + return aGrip.class; + } + + return aGrip.class + " " + VariablesView.getString(aGrip.preview.text); + }, + + ObjectWithURL: function(aGrip, {concise}) { + let result = aGrip.class; + let url = aGrip.preview.url; + if (!VariablesView.isFalsy({ value: url })) { + result += " \u2192 " + WebConsoleUtils.abbreviateSourceURL(url, + { onlyCropQuery: !concise }); + } + return result; + }, + + // Stringifier for any kind of object. + Object: function(aGrip, {concise}) { + if (concise) { + return aGrip.class; + } + + let {preview} = aGrip; + let props = []; + for (let key of Object.keys(preview.ownProperties || {})) { + let value = preview.ownProperties[key]; + let valueString = ""; + if (value.get) { + valueString = "Getter"; + } else if (value.set) { + valueString = "Setter"; + } else { + valueString = VariablesView.getString(value.value, { concise: true }); + } + props.push(key + ": " + valueString); + } + + for (let key of Object.keys(preview.safeGetterValues || {})) { + let value = preview.safeGetterValues[key]; + let valueString = VariablesView.getString(value.getterValue, + { concise: true }); + props.push(key + ": " + valueString); + } + + if (!props.length) { + return null; + } + + if (preview.ownPropertiesLength) { + let previewLength = Object.keys(preview.ownProperties).length; + let diff = preview.ownPropertiesLength - previewLength; + if (diff > 0) { + props.push(VariablesView.stringifiers._getNMoreString(diff)); + } + } + + let prefix = aGrip.class != "Object" ? aGrip.class + " " : ""; + return prefix + "{" + props.join(", ") + "}"; + }, // Object + + Error: function(aGrip, {concise}) { + let {preview} = aGrip; + let name = VariablesView.getString(preview.name, { noStringQuotes: true }); + if (concise) { + return name || aGrip.class; + } + + let msg = name + ": " + + VariablesView.getString(preview.message, { noStringQuotes: true }); + + if (!VariablesView.isFalsy({ value: preview.stack })) { + msg += "\n" + STR.GetStringFromName("variablesViewErrorStacktrace") + + "\n" + preview.stack; + } + + return msg; + }, + + DOMException: function(aGrip, {concise}) { + let {preview} = aGrip; + if (concise) { + return preview.name || aGrip.class; + } + + let msg = aGrip.class + " [" + preview.name + ": " + + VariablesView.getString(preview.message) + "\n" + + "code: " + preview.code + "\n" + + "nsresult: 0x" + (+preview.result).toString(16); + + if (preview.filename) { + msg += "\nlocation: " + preview.filename; + if (preview.lineNumber) { + msg += ":" + preview.lineNumber; + } + } + + return msg + "]"; + }, + + DOMEvent: function(aGrip, {concise}) { + let {preview} = aGrip; + if (!preview.type) { + return null; + } + + if (concise) { + return aGrip.class + " " + preview.type; + } + + let result = preview.type; + + if (preview.eventKind == "key" && preview.modifiers && + preview.modifiers.length) { + result += " " + preview.modifiers.join("-"); + } + + let props = []; + if (preview.target) { + let target = VariablesView.getString(preview.target, { concise: true }); + props.push("target: " + target); + } + + for (let prop in preview.properties) { + let value = preview.properties[prop]; + props.push(prop + ": " + VariablesView.getString(value, { concise: true })); + } + + return result + " {" + props.join(", ") + "}"; + }, // DOMEvent + + DOMNode: function(aGrip, {concise}) { + let {preview} = aGrip; + + switch (preview.nodeType) { + case Ci.nsIDOMNode.DOCUMENT_NODE: { + let location = WebConsoleUtils.abbreviateSourceURL(preview.location, + { onlyCropQuery: !concise }); + return aGrip.class + " \u2192 " + location; + } + + case Ci.nsIDOMNode.ATTRIBUTE_NODE: { + let value = VariablesView.getString(preview.value, { noStringQuotes: true }); + return preview.nodeName + '="' + escapeHTML(value) + '"'; + } + + case Ci.nsIDOMNode.TEXT_NODE: + return preview.nodeName + " " + + VariablesView.getString(preview.textContent); + + case Ci.nsIDOMNode.COMMENT_NODE: { + let comment = VariablesView.getString(preview.textContent, + { noStringQuotes: true }); + return ""; + } + + case Ci.nsIDOMNode.DOCUMENT_FRAGMENT_NODE: { + if (concise || !preview.childNodes) { + return aGrip.class + "[" + preview.childNodesLength + "]"; + } + let nodes = []; + for (let node of preview.childNodes) { + nodes.push(VariablesView.getString(node)); + } + if (nodes.length < preview.childNodesLength) { + let n = preview.childNodesLength - nodes.length; + nodes.push(VariablesView.stringifiers._getNMoreString(n)); + } + return aGrip.class + " [" + nodes.join(", ") + "]"; + } + + case Ci.nsIDOMNode.ELEMENT_NODE: { + let attrs = preview.attributes; + if (!concise) { + let n = 0, result = "<" + preview.nodeName; + for (let name in attrs) { + let value = VariablesView.getString(attrs[name], + { noStringQuotes: true }); + result += " " + name + '="' + escapeHTML(value) + '"'; + n++; + } + if (preview.attributesLength > n) { + result += " " + Scope.ellipsis; + } + return result + ">"; + } + + let result = "<" + preview.nodeName; + if (attrs.id) { + result += "#" + attrs.id; + } + return result + ">"; + } + + default: + return null; + } + }, // DOMNode +}; // VariablesView.stringifiers.byObjectKind + + +/** + * Get the "N more…" formatted string, given an N. This is used for displaying + * how many elements are not displayed in an object preview (eg. an array). + * + * @private + * @param number aNumber + * @return string + */ +VariablesView.stringifiers._getNMoreString = function(aNumber) { + let str = STR.GetStringFromName("variablesViewMoreObjects"); + return PluralForm.get(aNumber, str).replace("#1", aNumber); +}; + /** * Returns a custom class style for a grip. * @@ -3208,6 +3591,22 @@ let generateId = (function() { }; })(); +/** + * Escape some HTML special characters. We do not need full HTML serialization + * here, we just want to make strings safe to display in HTML attributes, for + * the stringifiers. + * + * @param string aString + * @return string + */ +function escapeHTML(aString) { + return aString.replace(/&/g, "&") + .replace(/"/g, """) + .replace(//g, ">"); +} + + /** * An Editable encapsulates the UI of an edit box that overlays a label, * allowing the user to edit the value. diff --git a/browser/devtools/webconsole/console-output.js b/browser/devtools/webconsole/console-output.js index 3ba7571fbe44..9724813e853b 100644 --- a/browser/devtools/webconsole/console-output.js +++ b/browser/devtools/webconsole/console-output.js @@ -997,10 +997,12 @@ Messages.Extended.prototype = Heritage.extend(Messages.Simple.prototype, } let result = this.document.createDocumentFragment(); - if (!isPrimitive || (!this._quoteStrings && typeof piece == "string")) { - result.textContent = piece; + if (isPrimitive) { + result.textContent = VariablesView.getString(piece, { + noStringQuotes: !this._quoteStrings, + }); } else { - result.textContent = VariablesView.getString(piece); + result.textContent = piece; } return result; @@ -1219,7 +1221,7 @@ Widgets.JSObject.prototype = Heritage.extend(Widgets.BaseWidget.prototype, _onClick: function() { this.output.openVariablesView({ - label: this.element.textContent, + label: VariablesView.getString(this.objectActor, { concise: true }), objectActor: this.objectActor, autofocus: true, }); @@ -1273,11 +1275,10 @@ Widgets.LongString.prototype = Heritage.extend(Widgets.BaseWidget.prototype, */ _renderString: function(str) { - if (this.message._quoteStrings) { - this.element.textContent = VariablesView.getString(str); - } else { - this.element.textContent = str; - } + this.element.textContent = VariablesView.getString(str, { + noStringQuotes: !this.message._quoteStrings, + noEllipsis: true, + }); }, /** diff --git a/browser/devtools/webconsole/webconsole.js b/browser/devtools/webconsole/webconsole.js index 3596285520fe..ab28e9903c67 100644 --- a/browser/devtools/webconsole/webconsole.js +++ b/browser/devtools/webconsole/webconsole.js @@ -1183,10 +1183,6 @@ WebConsoleFrame.prototype = { let clipboardArray = []; args.forEach((aValue) => { clipboardArray.push(VariablesView.getString(aValue)); - if (aValue && typeof aValue == "object" && - aValue.type == "longString") { - clipboardArray.push(l10n.getStr("longStringEllipsis")); - } }); clipboardText = clipboardArray.join(" "); break; @@ -3103,7 +3099,7 @@ JSTerm.prototype = { aAfterMessage._objectActors.add(helperResult.object.actor); } this.openVariablesView({ - label: VariablesView.getString(helperResult.object), + label: VariablesView.getString(helperResult.object, { concise: true }), objectActor: helperResult.object, }); break; diff --git a/browser/locales/en-US/chrome/browser/devtools/debugger.properties b/browser/locales/en-US/chrome/browser/devtools/debugger.properties index 0aa508cf5d9e..9aa94b8ffaa7 100644 --- a/browser/locales/en-US/chrome/browser/devtools/debugger.properties +++ b/browser/locales/en-US/chrome/browser/devtools/debugger.properties @@ -202,8 +202,8 @@ loadingText=Loading\u2026 # viewer when there is an error loading a file errorLoadingText=Error loading source:\n -# LOCALIZATION NOTE (emptyStackText): The text that is displayed in the watch -# expressions list to add a new item. +# LOCALIZATION NOTE (addWatchExpressionText): The text that is displayed in the +# watch expressions list to add a new item. addWatchExpressionText=Add watch expression # LOCALIZATION NOTE (emptyVariablesText): The text that is displayed in the @@ -225,6 +225,19 @@ watchExpressionsScopeLabel=Watch expressions # the global scope. globalScopeLabel=Global +# LOCALIZATION NOTE (variablesViewErrorStacktrace): This is the text that is +# shown before the stack trace in an error. +variablesViewErrorStacktrace=Stack trace: + +# LOCALIZATION NOTE (variablesViewMoreObjects): the text that is displayed +# when you have an object preview that does not show all of the elements. At the end of the list +# you see "N more..." in the web console output. +# This is a semi-colon list of plural forms. +# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals +# #1 number of remaining items in the object +# example: 3 more… +variablesViewMoreObjects=#1 more…;#1 more… + # LOCALIZATION NOTE (variablesEditableNameTooltip): The text that is displayed # in the variables list on an item with an editable name. variablesEditableNameTooltip=Double click to edit diff --git a/toolkit/devtools/webconsole/utils.js b/toolkit/devtools/webconsole/utils.js index b77bee53faec..6a3ee13efefc 100644 --- a/toolkit/devtools/webconsole/utils.js +++ b/toolkit/devtools/webconsole/utils.js @@ -188,12 +188,18 @@ let WebConsoleUtils = { * * @param string aSourceURL * The source URL to shorten. + * @param object [aOptions] + * Options: + * - onlyCropQuery: boolean that tells if the URL abbreviation function + * should only remove the query parameters and the hash fragment from + * the given URL. * @return string * The abbreviated form of the source URL. */ - abbreviateSourceURL: function WCU_abbreviateSourceURL(aSourceURL) + abbreviateSourceURL: + function WCU_abbreviateSourceURL(aSourceURL, aOptions = {}) { - if (aSourceURL.substr(0, 5) == "data:") { + if (!aOptions.onlyCropQuery && aSourceURL.substr(0, 5) == "data:") { let commaIndex = aSourceURL.indexOf(","); if (commaIndex > -1) { aSourceURL = "data:" + aSourceURL.substring(commaIndex + 1); @@ -214,13 +220,15 @@ let WebConsoleUtils = { // Remove a trailing "/". if (aSourceURL[aSourceURL.length - 1] == "/") { - aSourceURL = aSourceURL.substring(0, aSourceURL.length - 1); + aSourceURL = aSourceURL.replace(/\/+$/, ""); } // Remove all but the last path component. - let slashIndex = aSourceURL.lastIndexOf("/"); - if (slashIndex > -1) { - aSourceURL = aSourceURL.substring(slashIndex + 1); + if (!aOptions.onlyCropQuery) { + let slashIndex = aSourceURL.lastIndexOf("/"); + if (slashIndex > -1) { + aSourceURL = aSourceURL.substring(slashIndex + 1); + } } return aSourceURL; From 838f1610c87742a8408db73b9d07192447256444 Mon Sep 17 00:00:00 2001 From: Mihai Sucan Date: Thu, 19 Dec 2013 23:07:54 +0200 Subject: [PATCH 09/18] Bug 843004 - Part 4: Tests for pretty output of objects; r=nfitzgerald --HG-- rename : browser/devtools/webconsole/test/browser_webconsole_bug_598357_jsterm_output.js => browser/devtools/webconsole/test/browser_webconsole_output_01.js --- .../test/browser_dbg_closure-inspection.js | 4 +- .../test/browser_dbg_pause-exceptions-01.js | 4 +- .../test/browser_dbg_pause-exceptions-02.js | 2 +- .../browser_dbg_variables-view-edit-value.js | 6 +- ..._dbg_variables-view-frame-parameters-01.js | 6 +- ..._dbg_variables-view-frame-parameters-02.js | 12 +- ..._dbg_variables-view-frame-parameters-03.js | 12 +- .../browser_dbg_variables-view-frame-with.js | 5 +- ...r_dbg_variables-view-large-array-buffer.js | 2 +- .../test/browser_dbg_variables-view-webidl.js | 17 +- browser/devtools/webconsole/test/browser.ini | 10 +- ..._865871_variables_view_close_on_esc_key.js | 6 +- ..._bug_869003_inspect_cross_domain_object.js | 2 +- .../test/browser_console_consolejsm_output.js | 4 +- .../browser_console_log_inspectable_object.js | 2 +- .../test/browser_console_native_getters.js | 4 +- .../test/browser_console_variables_view.js | 3 +- ..._console_variables_view_while_debugging.js | 3 +- ...les_view_while_debugging_and_inspecting.js | 3 +- .../webconsole/test/browser_jsterm_inspect.js | 2 +- .../test/browser_result_format_as_string.js | 8 +- ...eactivateHUDForContext_unfocused_window.js | 5 +- ...ser_webconsole_bug_598357_jsterm_output.js | 267 ------------------ ...e_bug_651501_document_body_autocomplete.js | 19 +- ...e_bug_653531_highlighter_console_helper.js | 3 +- ...owser_webconsole_bug_659907_console_dir.js | 6 +- .../browser_webconsole_closure_inspection.js | 2 +- .../test/browser_webconsole_jsterm.js | 4 +- .../test/browser_webconsole_output_01.js | 146 ++++++++++ .../test/browser_webconsole_output_02.js | 161 +++++++++++ .../test/browser_webconsole_output_03.js | 164 +++++++++++ .../test/browser_webconsole_output_04.js | 120 ++++++++ .../test/browser_webconsole_output_events.js | 60 ++++ browser/devtools/webconsole/test/head.js | 173 +++++++++++- .../test/test-console-output-02.html | 61 ++++ .../test/test-console-output-03.html | 30 ++ .../test/test-console-output-04.html | 77 +++++ .../test/test-console-output-events.html | 23 ++ browser/devtools/webconsole/webconsole.js | 1 + 39 files changed, 1094 insertions(+), 345 deletions(-) delete mode 100644 browser/devtools/webconsole/test/browser_webconsole_bug_598357_jsterm_output.js create mode 100644 browser/devtools/webconsole/test/browser_webconsole_output_01.js create mode 100644 browser/devtools/webconsole/test/browser_webconsole_output_02.js create mode 100644 browser/devtools/webconsole/test/browser_webconsole_output_03.js create mode 100644 browser/devtools/webconsole/test/browser_webconsole_output_04.js create mode 100644 browser/devtools/webconsole/test/browser_webconsole_output_events.js create mode 100644 browser/devtools/webconsole/test/test-console-output-02.html create mode 100644 browser/devtools/webconsole/test/test-console-output-03.html create mode 100644 browser/devtools/webconsole/test/test-console-output-04.html create mode 100644 browser/devtools/webconsole/test/test-console-output-events.html diff --git a/browser/devtools/debugger/test/browser_dbg_closure-inspection.js b/browser/devtools/debugger/test/browser_dbg_closure-inspection.js index 1acc1d1fc765..f04c12837d22 100644 --- a/browser/devtools/debugger/test/browser_dbg_closure-inspection.js +++ b/browser/devtools/debugger/test/browser_dbg_closure-inspection.js @@ -77,13 +77,13 @@ function test() { .getAttribute("value"), "getName", "Should have the right property name for 'getName' in person."); is(personNode.get("getName").target.querySelector(".value") - .getAttribute("value"), "Function", + .getAttribute("value"), "_pfactory/<.getName()", "'getName' in person should have the right value."); is(personNode.get("getFoo").target.querySelector(".name") .getAttribute("value"), "getFoo", "Should have the right property name for 'getFoo' in person."); is(personNode.get("getFoo").target.querySelector(".value") - .getAttribute("value"), "Function", + .getAttribute("value"), "_pfactory/<.getFoo()", "'getFoo' in person should have the right value."); // Expand the function nodes. This causes their properties to be diff --git a/browser/devtools/debugger/test/browser_dbg_pause-exceptions-01.js b/browser/devtools/debugger/test/browser_dbg_pause-exceptions-01.js index 407e701b64b3..b51c155a64c7 100644 --- a/browser/devtools/debugger/test/browser_dbg_pause-exceptions-01.js +++ b/browser/devtools/debugger/test/browser_dbg_pause-exceptions-01.js @@ -58,7 +58,7 @@ function testPauseOnExceptionsDisabled() { is(innerNodes[0].querySelector(".name").getAttribute("value"), "this", "Should have the right property name for 'this'."); - is(innerNodes[0].querySelector(".value").getAttribute("value"), "HTMLButtonElement", + is(innerNodes[0].querySelector(".value").getAttribute("value"), "