Closed
Bug 745303
Opened 12 years ago
Closed 12 years ago
Let Panorama use the thumbnail service
Categories
(Firefox Graveyard :: Panorama, defect)
Firefox Graveyard
Panorama
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 14
People
(Reporter: ttaubert, Assigned: ttaubert)
References
Details
Attachments
(2 files, 2 obsolete files)
19.54 KB,
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
37.62 KB,
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
Panorama should use the thumbnail service to display thumbnails for not-yet-restored tabs. We can rip out the whole thumbnail storage and other code parts with this. Also, this will fix bug 738128.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #614948 -
Attachment is obsolete: true
Assignee | ||
Comment 4•12 years ago
|
||
Comment on attachment 614949 [details] [diff] [review] Part 2 - Correct Panorama tests Explaining some changes: >--- a/browser/components/tabview/test/Makefile.in > browser_tabview_bug596781.js \ >- browser_tabview_bug597248.js \ > browser_tabview_bug597360.js \ Removing the test for bug 597248. No need to make sure Panorama's thumbnail cache is solid anymore. > browser_tabview_bug626791.js \ >- browser_tabview_bug627288.js \ > browser_tabview_bug627736.js \ Removing test for bug 627288 because TabItem.showCachedData() retrieves its data now from TabItem.getTabState() which is never stale. Title and URL (and therefore the thumbnail) are always up-to-date. > browser_tabview_bug673729.js \ >- browser_tabview_bug677310.js \ > browser_tabview_bug678374.js \ Removing test for bug 677310 because we can't control whether all thumbnails have been captured before transitioning into private browsing mode. This is okay since we shouldn't execute any sync operations and most thumbnails should have been stored anyway before entering PB mode.
Attachment #614949 -
Flags: review?(dietrich)
Assignee | ||
Comment 5•12 years ago
|
||
And again explaining some changes: >--- a/browser/components/tabview/content.js There's a lot of stuff we can remove from the Panorama content script. The majority of the code was there to figure out the storage policy for thumbnails. >diff --git a/browser/components/tabview/storagePolicy.js b/browser/components/tabview/storagePolicy.js >diff --git a/browser/components/tabview/thumbnailStorage.js b/browser/components/tabview/thumbnailStorage.js We don't the thumbnail storage and its storage policy anymore. >--- a/browser/components/tabview/tabitems.js > _reconnect: function TabItem__reconnect(options) { > Utils.assertThrow(!this._reconnected, "shouldn't already be reconnected"); > Utils.assertThrow(this.tab, "should have a xul:tab"); > > let tabData = Storage.getTabData(this.tab); > let groupItem; > > if (tabData && TabItems.storageSanity(tabData)) { >- this.loadThumbnail(); >+ // Show the cached data while we're waiting for the tabItem to be updated. >+ // If the tab isn't restored yet this acts as a placeholder until it is. >+ this.showCachedData(); > We never show stale data, we just create a placeholder while the tab is loading/restoring or waiting to be restored. So we can just unconditionally call showCachedData() because this gets removed when the TabItem is updated.
Attachment #614953 -
Attachment is obsolete: true
Attachment #614958 -
Flags: review?(dietrich)
Comment 6•12 years ago
|
||
Comment on attachment 614958 [details] [diff] [review] Part 1 - Make Panorama use the thumbnail service Review of attachment 614958 [details] [diff] [review]: ----------------------------------------------------------------- yay code removal. boo on making the tooltip changes on an unrelated patch :P but they look ok, so r=me.
Attachment #614958 -
Flags: review?(dietrich) → review+
Updated•12 years ago
|
Attachment #614949 -
Flags: review?(dietrich) → review+
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Dietrich Ayala (:dietrich) from comment #6) > yay code removal. boo on making the tooltip changes on an unrelated patch :P > but they look ok, so r=me. Sorry, I couldn't resist :)
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d42051ed9a81 https://hg.mozilla.org/integration/fx-team/rev/15387b04ebf2
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 14
Assignee | ||
Comment 9•12 years ago
|
||
Forgot to remove the now empty TabItem.loadThumbnail() method: https://hg.mozilla.org/integration/fx-team/rev/c2ee16305bc6
Assignee | ||
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d42051ed9a81 https://hg.mozilla.org/mozilla-central/rev/15387b04ebf2 https://hg.mozilla.org/mozilla-central/rev/c2ee16305bc6
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 11•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/06438370bd3f
Assignee | ||
Comment 12•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/06438370bd3f
Updated•8 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•