305986 - Show thumbnail for large image pages

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, enhancement)

Description

[Hixie (not reading bugmail)]

------- Bug 304561 Comment #16 From Ian 'Hixie' Hickson -------

Is there any reason we can't do what you suggested above, vlad, and "simply"
generate a resized version of the image and set that as the favicon instead of
resizing the big one each time? We could also use that image as the favicon when
you bookmark an image, if we did that, without putting multi-megabyte images
into the bookmarks file.


------- Bug 304561 #17 From Vladimir Vukicevic -------

That sounds great; let me know how I generate a resized version of the image and
set it as the src for an <img> :)


------- Bug 304561 Comment #18 From Ian 'Hixie' Hickson -------

Well, one inefficient way would be to draw the image to a canvas and then grab a
data: URI out of the canvas... But I'm not suggesting doing it that way. Can't
imagelib provide some sort of convenient API to do something like this?


------- Bug 304561 Comment #19 From Vladimir Vukicevic -------

Actually, that would be a pretty efficient way; the problem is that we have no
way of encoding an image back into data for creating that data URI.  We'll have
something in place for gecko1.9/future, but not for ffox 1.5.

Comment 1

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Hijacking this bug for SeaMonkey too...

Comment 2

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: SeaMonkey patch

This preserves the ability to fall back to the old code in non-canvas-enabled builds.

Comment 3

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: Firefox patch

Same deal as the SeaMonkey patch, but untested as I don't build Firefox.  Can someone test this before I request a review?  If it doesn't work, it's most likely

+                    var tabImg = document.getAnonymousNodes(this.mTab)[0].firstChild;

which needs to get the actual image in the tab.

Comment 4

[[e-mail]]

Comment on attachment 226751 [details] [diff] [review]
SeaMonkey patch

>+                    var tabImg = document.getAnonymousNodes(this.mTab)[0].firstChild;
This makes unrealistic assumptions about the tab bindings.

>+                    var theStyle = window.getComputedStyle(tabImg, null);
>+                    var w = parseInt(theStyle.width);
>+                    var h = parseInt(theStyle.height);
>+                    canvas.width = w;
>+                    canvas.height = h;
IMHO you should use .boxObject.width/height.

>+                    if (req && req.image.width <= sz && req.image.height <= sz)
You should always verify the request of an image document.

Comment 5

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: SM patch, v2

More robust tab-image-finding, use boxObject dimensions, verify the image request didn't fail before using it.

Comment 6

[[e-mail]]

Comment on attachment 232625 [details] [diff] [review]
SM patch, v2

>+                      var tabImg = this.mTab.getElementsByTagName("img")[0];
I don't think this will work at all, because the image is anonymous. It's not the only option, but maybe you meant to combine this with your first version.

Note that in the them I'm thinking of, the tab binding looks like this:
<content>
  <xul:box class="tab-outer-box" flex="1">
    <xul:box class="tab-middle-box" flex="1">
      <xul:box class="tab-inner-box" flex="1" align="center">
        <xul:image class="tab-image" xbl:inherits="busy,src=image,validate"/>
        <xul:box class="tab-focus-box" flex="1">
          <xul:text flex="1" xbl:inherits="value=label,crop,accesskey" crop="right" class="tab-text"/>
          <children/>
        </xul:box>
        <children includes="image"/>
      </xul:box>
    </xul:box>
  </xul:box>
</content>
While I'm prepared to adapt the binding to work with tab thumbnails, document.getAnonymousNodes(this.mTab)[0].firstChild has no hope of working.

Comment 7

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #6)
> (From update of attachment 232625 [details] [diff] [review] [edit])
> >+                      var tabImg = this.mTab.getElementsByTagName("img")[0];
> I don't think this will work at all, because the image is anonymous. It's not
> the only option, but maybe you meant to combine this with your first version.

I must not have recompiled when I tested it - as you said, it doesn't work at all.

Let's go with:
                      var tabImg = this.mTab.getAnonymousElementsByAttribute("class", "tab-image");

...which does work.

Comment 8

[[e-mail]]

(In reply to comment #7)
>var tabImg = this.mTab.getAnonymousElementsByAttribute("class", "tab-image");
I'd prefer you set an anonid attribute, in case we need to change the class.

Comment 9

[[e-mail]]

Comment on attachment 232625 [details] [diff] [review]
SM patch, v2

As per previous comments.

Comment 10

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: SM patch, v3

Comment 11

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Fixed in SeaMonkey.

Comment 12

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: Firefox patch

Comment 13

[Asaf Romano (gone)]

Comment on attachment 238569 [details] [diff] [review]
Firefox patch

As I noted on IRC, globalBindings.xml of each theme overrides this too. On current trunk, only Pinstripe has an extended binding, but as soon as the new theme lands on trunk, this won't affect winstipre as well.

Also, you may want to replace the try..catch blocks with #ifdef MOZ_ENABLE_CANVAS.

Comment 14

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: firefox patch

Fixes pinstripe.  I prefer try/catch to #ifdef.

Comment 15

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 238828 [details] [diff] [review]
firefox patch

Ooops.

Comment 16

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: firefox patch

Comment 17

[Asaf Romano (gone)]

Comment on attachment 238829 [details] [diff] [review]
firefox patch

Please do #ifdef this for toolkit, also, you need to patch the updated overriding bindings of winstripe and pinstripe.

Comment 18

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Someone else can take care of the Firefox version.

Comment 19

[Andrew Schultz]

I'm going to declare this FIXED for SM.  If someone wants to port bits to firefox, please file a new bug.