Last Comment Bug 725268 - [Page Thumbnails] Reduce API provided by PageThumbs.jsm
: [Page Thumbnails] Reduce API provided by PageThumbs.jsm
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 13
Assigned To: Tim Taubert [:ttaubert]
: Dão Gottwald [:dao]
Depends on:
  Show dependency treegraph
Reported: 2012-02-08 04:37 PST by Tim Taubert [:ttaubert]
Modified: 2012-02-10 07:26 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v1 (6.93 KB, patch)
2012-02-08 05:03 PST, Tim Taubert [:ttaubert]
dietrich: review+
Details | Diff | Splinter Review

Description User image Tim Taubert [:ttaubert] 2012-02-08 04:37:50 PST
From bug 720838 comment #12:

> I agree that adding a captureAndStore(aBrowser) method on PageThumbs would
> be useful, to reduce unnecessary complexity for the primary use of this API
> (browser-thumbnails.js).
> Exposing store() as part of the PageThumbs API doesn't seem justified at the
> moment. Who would want to use an arbitrary key or save data from another
> source? This could actually lead to trouble if addons end up improperly
> storing data. I can see capture()'s potentially utility, though, and keeping
> it doesn't seem harmful.
Comment 1 User image Tim Taubert [:ttaubert] 2012-02-08 05:03:35 PST
Created attachment 595382 [details] [diff] [review]
patch v1

Implements the proposed API.

1) capture() remains untouched.
2) captureAndStore(aBrowser[, aCallback]) was added
3) store() got removed
Comment 2 User image Dietrich Ayala (:dietrich) 2012-02-08 12:09:48 PST
Comment on attachment 595382 [details] [diff] [review]
patch v1

Review of attachment 595382 [details] [diff] [review]:

::: browser/components/thumbnails/PageThumbs.jsm
@@ +110,3 @@
>     */
> +  captureAndStore: function PageThumbs_captureAndStore(aBrowser, aCallback) {
> +    this.capture(aBrowser.contentWindow, function (aData) {

aData too ambiguous, rename to reflect actual type.
Comment 3 User image Tim Taubert [:ttaubert] 2012-02-09 02:41:44 PST
Comment 4 User image Tim Taubert [:ttaubert] 2012-02-09 02:52:18 PST
Backed out because I accidentally slipped in changes from another patch:

Pushed again:
Comment 5 User image Tim Taubert [:ttaubert] 2012-02-10 07:25:26 PST

Note You need to log in before you can comment on or make changes to this bug.