Provide image scaling for embedded images

RESOLVED FIXED in Thunderbird 28.0

Status

Thunderbird
Mail Window Front End
--
enhancement
RESOLVED FIXED
8 years ago
8 days ago

People

(Reporter: Dale, Assigned: alta88)

Tracking

(Depends on: 2 bugs, Blocks: 2 bugs)

Trunk
Thunderbird 28.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tb31features])

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.5) Gecko/20091204 Thunderbird/3.0

It's pretty simple. Sometimes an image in the e-mail viewing window actually scales down horizontally to the size of the window and gives you the option to click on it to view at 100% zoom. Sometimes it does not... more often not.

Makes for a pretty unreliable feature.

Reproducible: Sometimes

Steps to Reproduce:
1. Open an e-mail with a large (too big to fit window) image.

2. Get lucky and it might resize to fit the window horizontally.

3. Try clicking on the e-mail over and over again, resize the application window, call it names, etc.


Actual Results:  
Give up and just open in Photoshop or other external previewer because it probably won't resize but you might get lucky.

Expected Results:  
What it is suppose to do.... resize the image horizontally to fit the e-mail viewing area.

I just downloaded this and I am using the previous account info from Thunderbird 2.
Can you share one email with us that behaves that way ?

Do you have the issue in -safe-mode (http://kb.mozillazine.org/Safe_mode) ?

When this happens , are there any error messages in Tools -> Error console ?
Component: General → Mail Window Front End
QA Contact: general → front-end
(Reporter)

Comment 2

8 years ago
No error messages, still happening in safe mode. I am sending one that is doing it right now to: ludovic@mozillamessaging.com.
(Reporter)

Comment 3

8 years ago
This comes up on the SENT message that I just sent.... it comes up 8 times at once (I am in safe mode):



Warning: Cannot specify value for internal property.  Error in parsing value for '-x-system-font'.  Declaration dropped.
Source File: mailbox:///Users/station4/Library/Thunderbird/Profiles/pp05zwlm.default/Mail/Local%20Folders/Sent?number=-1965717595
Line: 0
(Reporter)

Comment 4

8 years ago
For some reason I get no errors on original e-mail
I have receied a forwarded email which is forwarded.

Html code of it is :
<table class="moz-email-headers-table" border="0" cellpadding="0"
 cellspacing="0">
  <tbody>
    <tr>
      <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject: </th>
      <td>Fwd: willows ad</td>
    </tr>
    <tr>
      <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date: </th>
      <td>Thu, 3 Dec 2009 12:20:13 -0600</td>
    </tr>
    <tr>
      <th align="RIGHT" nowrap="nowrap" valign="BASELINE">From: </th>
      <td>Judy <a class="moz-txt-link-rfc2396E" href="mailto:judy@courierjournal.net">&lt;judy@courierjournal.net&gt;</a></td>
    </tr>
    <tr>
      <th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th>
      <td>Courier Journal Art Department

<a class="moz-txt-link-rfc2396E" href="mailto:artdepartment@courierjournal.net">&lt;artdepartment@courierjournal.net&gt;</a></td>
    </tr>
  </tbody>
</table>
<br>
<br>
<br>
<div><br>
<div>Begin forwarded message:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
  <div style="margin: 0px;"><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none; color: rgb(0, 0, 0);"
 color="#000000" face="Helvetica" size="3"><b>From: </b></font><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none;"
 face="Helvetica" size="3">Heather Fuller &lt;<a moz-do-not-send="true"
 href="mailto:willowsinfo@gmail.com">willowsinfo@gmail.com</a>&gt;</font></div>
  <div style="margin: 0px;"><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none; color: rgb(0, 0, 0);"
 color="#000000" face="Helvetica" size="3"><b>Date: </b></font><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none;"
 face="Helvetica" size="3">December 3, 2009 10:19:08 AM CST</font></div>
  <div style="margin: 0px;"><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none; color: rgb(0, 0, 0);"
 color="#000000" face="Helvetica" size="3"><b>To: </b></font><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none;"
 face="Helvetica" size="3"><a moz-do-not-send="true"
 href="mailto:judy@courierjournal.net">judy@courierjournal.net</a></font></div>
  <div style="margin: 0px;"><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none; color: rgb(0, 0, 0);"
 color="#000000" face="Helvetica" size="3"><b>Subject: </b></font><font
 style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; font-size: 12px; line-height: normal; font-size-adjust: none; font-stretch: normal; -x-system-font: none;"
 face="Helvetica" size="3"><b>willows ad</b></font></div>
  <div style="margin: 0px; min-height: 14px;"><br>
  </div>
Here's the pic for the ad. <br>
Thanks<br>
Amber<br>
The Willows Day Spa<br>
  <img src="cid:part1.06020006.02080100@courierjournal.net"></blockquote>

</div>
<br>
<br>
</body>
</html>

Comment 6

8 years ago
A little background:
That feature was added in bug 372080 and was designed to work for attached
images only, and not inline images.
The example mail is using an inline image, which has never used re-scaling for me.
(see my comments in that bug)

You can see the feature in action if you send yourself a large attached image.
There is also a hidden pref involved (browser.enable_automatic_image_resizing)
which seems to control the initial state of the re-scale.

So, unless someone would like to use this bug to extend the feature to inline
images, this bug should be resolved as invalid.
(Reporter)

Comment 7

8 years ago
It really needs to be implemented as a feature then if possible because in real world use it seems like the feature doesn't work correctly.

For instance where I work I receive e-mail from many clients and if only some of them send pictures as actual attachments then it makes the feature kind of pointless since I am going to have to open Photoshop or Preview anyway for the other "inline" attachments.

But some is better than none I suppose.

Anyway, thank you for the prompt responses to my "bug" report.

Comment 8

8 years ago
I take your comments as permission to change this bug to an enhancement request.

Summary:
There is no easy way to view an over-sized image that has been inserted into an HTML composition.
Bug 372080 provided that for images sent as attachments, but I would argue that
if a new user is sending an HTML message, what more discoverable way is there
than to well, Insert Image.

I think that a right-click context menu item would suffice.
Severity: major → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X → All
Summary: Thunderbird 3 image scaling doesn't work sometimes. → Provide image scaling for embedded images
Duplicate of this bug: 675683
Duplicate of this bug: 674206

Updated

6 years ago
Duplicate of this bug: 684357

Updated

6 years ago
Duplicate of this bug: 682163

Updated

6 years ago
Hardware: x86_64 → All
Version: unspecified → Trunk

Comment 13

6 years ago
It also happens in Seamonkey.  2.1 works fine (earlier versions worked too), anything higher  than 2.1 doesn't work at all. Since it has been working for a long time, this doesn't sound like an "enhancement"
how has the code changed for this since 2.1?
See Also: → bug 641438
See Also: → bug 740772
(Assignee)

Comment 14

4 years ago
Auto shrink to fit should be the default for all <img> elements, and should behave just like inline image attachments do currently when overflowing the messagepane/window.  Brief spec is:

1. The current restriction of this functionality on <img> due to class = moz-attached-image would be removed.
2. Current moz-attached-image css would apply to all <img>.
3. The pref "mail.enable_automatic_image_resizing" would be removed (as would associated code in mimemoz2.cpp).  It's easy enough to click to get original size.
4. This would apply to all <img> in mail, nntp, feed (summary only, not web page) content.

Blake, if you agree with the above I can do a patch.
Flags: needinfo?(bwinton)
Sounds good to me!  Be careful that we don't expand images that don't take up the full width, though.  :)

Thanks,
Blake.
Flags: needinfo?(bwinton)
(Assignee)

Comment 16

4 years ago
The existing methods already handle that.  The one thing is link images; I think in messagepane it's more relevant to size the image.  So if an image is in zoom mode, a right click context will be needed to Open in Browser (if it's not autoshrunk, it will behave as a usual link click).
(Assignee)

Comment 17

4 years ago
Created attachment 818013 [details] [diff] [review]
imageScaling.patch
Assignee: nobody → alta88
Attachment #818013 - Flags: review?(bwinton)
(Assignee)

Comment 18

4 years ago
Created attachment 818107 [details] [diff] [review]
imageScaling.patch


some tweaks.
Attachment #818013 - Attachment is obsolete: true
Attachment #818013 - Flags: review?(bwinton)
Attachment #818107 - Flags: review?(bwinton)
(Assignee)

Comment 19

4 years ago
Created attachment 818502 [details] [diff] [review]
imageScaling.patch


some images may hardcode a height attribute, use height: auto to ensure proportional scaling.
Attachment #818107 - Attachment is obsolete: true
Attachment #818107 - Flags: review?(bwinton)
Attachment #818502 - Flags: review?(bwinton)

Comment 20

4 years ago
Thanks for doing this. I ran with this in a local build and tested a variety of inline images. Worked fine except for one little nit. If you have an image with a natural width high enough to cause overflow, but an inline style that shrinks the image, then the re-size widget appears but is ineffective. Maybe you could avoid that by using "computed style width" instead of naturalwidth to determine if it re-sizable. I'm sure this is an edge case, but could cause confusion for some.
(Assignee)

Comment 21

4 years ago
thanks for testing joe.  the solution to the inline style case is not immediately clear; changing naturalWidth to clientWidth solves the problem on initial display, but results in bad behavior on resizing, as clientWidth is alternately reported as either the style width or actual width (seems like a deeper bug) and results in constant scrollbar on/off on each resize event if a true shrink is needed per messagepane size.
(Assignee)

Comment 22

4 years ago
Created attachment 819904 [details] [diff] [review]
imageScaling.patch


this should fix inline styles and resizing.  the extreme tester will note that if a resize makes pane and styled image exactly pixel equal, zoom will appear; nothing to be done as if = is used the problem reappears.  also note that !important is added to enforce the shrinktofit (a really stubborn style can add its own).
Attachment #818502 - Attachment is obsolete: true
Attachment #818502 - Flags: review?(bwinton)
Attachment #819904 - Flags: review?(bwinton)
(Assignee)

Comment 23

4 years ago
Created attachment 820045 [details] [diff] [review]
imageScaling.patch


handle the case where the image is already shrunk (width: 100%) to allow zoom in to naturalWidth, to have a consistency of experience regardless of inline css.
Attachment #819904 - Attachment is obsolete: true
Attachment #819904 - Flags: review?(bwinton)
Attachment #820045 - Flags: review?(bwinton)

Updated

4 years ago
Blocks: 360488
(Assignee)

Comment 24

4 years ago
ping?
(Sorry, new computer can't build Thunderbird yet!  I'll try to get to it on the old computer this weekend!)
Comment on attachment 820045 [details] [diff] [review]
imageScaling.patch

Review of attachment 820045 [details] [diff] [review]:
-----------------------------------------------------------------

Okay, so aside from the things I mention below, I like this patch.
r=me, with nits fixed, and question answered!  ;)

Thanks,
Blake.

::: mail/base/content/contentAreaClick.js
@@ +54,5 @@
>    }
>  
>    function messagePaneOnResize(aEvent)
>    {
> +    // Scale any overflowing images, exclude http content.  

Please remove the trailing spaces here.
(And the other places they occur.  :)

@@ +57,5 @@
>    {
> +    // Scale any overflowing images, exclude http content.  
> +    let browser = getBrowser();
> +    let doc = browser && browser.contentDocument ? browser.contentDocument : null;
> +    let imgs = doc && !doc.URL.startsWith("http") ? doc.images : [];

The use of doc.images here is kinda neat!  :)

@@ +63,3 @@
>      {
> +      if ((img.clientWidth + doc.body.clientWidth - doc.body.offsetWidth) >=
> +             doc.body.clientWidth &&

I think it would be easier to check "img.clientWidth - doc.body.offsetWidth >= 0"…  (Removing doc.body.clientWidth from both sides of the inequality.)

@@ +67,5 @@
>        {
> +        if (img.getAttribute("shrinktofit") == "false")
> +          img.setAttribute("overflowing", true);
> +        else
> +          img.setAttribute("isshrunk", true);

I sort of want to combine these two into a single attribute, called something like "toobig".  (Or, you know, something better than that. ;)

::: mail/themes/linux/mail/messageBody.css
@@ +79,4 @@
>    image-orientation: from-image;
>  }
>  
> +img[overflowing="true"] { 

If we merged the attributes, this would change to something along the lines of:
img[toobig="true"]:not([shrinktofit]) {
and the next one would be:
img[toobig="true"][shrinktofit] {

::: mailnews/mailnews.js
@@ +753,3 @@
>  // automatically scale attached images that are displayed inline
>  pref("mail.enable_automatic_image_resizing", true);
>  

Why was this change made?
Attachment #820045 - Flags: review?(bwinton) → review+
(Assignee)

Comment 27

4 years ago
(In reply to Blake Winton (:bwinton) from comment #26)
> Comment on attachment 820045 [details] [diff] [review]
> imageScaling.patch
> 
> Review of attachment 820045 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Okay, so aside from the things I mention below, I like this patch.
> r=me, with nits fixed, and question answered!  ;)
> 
> Thanks,
> Blake.
> 
> ::: mail/base/content/contentAreaClick.js
> @@ +54,5 @@
> >    }
> >  
> >    function messagePaneOnResize(aEvent)
> >    {
> > +    // Scale any overflowing images, exclude http content.  
> 
> Please remove the trailing spaces here.
> (And the other places they occur.  :)
> 

done.

> @@ +57,5 @@
> >    {
> > +    // Scale any overflowing images, exclude http content.  
> > +    let browser = getBrowser();
> > +    let doc = browser && browser.contentDocument ? browser.contentDocument : null;
> > +    let imgs = doc && !doc.URL.startsWith("http") ? doc.images : [];
> 
> The use of doc.images here is kinda neat!  :)
> 

sure, let the dom devs do the work..

> @@ +63,3 @@
> >      {
> > +      if ((img.clientWidth + doc.body.clientWidth - doc.body.offsetWidth) >=
> > +             doc.body.clientWidth &&
> 
> I think it would be easier to check "img.clientWidth - doc.body.offsetWidth
> >= 0"…  (Removing doc.body.clientWidth from both sides of the inequality.)
> 

"I could have been a polynomial!"

> @@ +67,5 @@
> >        {
> > +        if (img.getAttribute("shrinktofit") == "false")
> > +          img.setAttribute("overflowing", true);
> > +        else
> > +          img.setAttribute("isshrunk", true);
> 
> I sort of want to combine these two into a single attribute, called
> something like "toobig".  (Or, you know, something better than that. ;)
> 
> ::: mail/themes/linux/mail/messageBody.css
> @@ +79,4 @@
> >    image-orientation: from-image;
> >  }
> >  
> > +img[overflowing="true"] { 
> 
> If we merged the attributes, this would change to something along the lines
> of:
> img[toobig="true"]:not([shrinktofit]) {
> and the next one would be:
> img[toobig="true"][shrinktofit] {
> 

done.

> ::: mailnews/mailnews.js
> @@ +753,3 @@
> >  // automatically scale attached images that are displayed inline
> >  pref("mail.enable_automatic_image_resizing", true);
> >  
> 
> Why was this change made?

that pref is only for img attachments inline, used by the mime emitter.  all img get the same treatment now so it's superfluous.  so only suite/ needs it.

by the way, if suite takes this patch, they should remove the pref and also clean up the code here:
http://mxr.mozilla.org/com m-central/source/mailnews/mime/src/mimemoz2.cpp#1217

imo, the class can be kept as it can't hurt to identify img attachments for whoever might need it.
(Assignee)

Comment 28

4 years ago
Created attachment 830210 [details] [diff] [review]
imageScaling.patch


updated for comments.
Attachment #820045 - Attachment is obsolete: true
Attachment #830210 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Comment 29

4 years ago
Created attachment 830240 [details] [diff] [review]
imageScaling.patch


tiny tweak to handle broken images.
Attachment #830210 - Attachment is obsolete: true
Attachment #830240 - Flags: review+
https://hg.mozilla.org/comm-central/rev/e78d12404a63
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 28.0
Blocks: 777679

Updated

4 years ago
Blocks: 939540
(Assignee)

Updated

3 years ago
Whiteboard: [tb31features]
No longer blocks: 777679
Depends on: 372080
Ian, does SeaMonkey want this? Nice feature, simple patch...
Which would allow us to do the cleanup mentioned below...

(In reply to alta88 from comment #27)
> by the way, if suite takes this patch, they should remove the pref and also
> clean up the code here:
> http://mxr.mozilla.org/comm-central/source/mailnews/mime/src/mimemoz2.cpp#1217

https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/mimemoz2.cpp#1107-1115
...and some other spots related to that.

> imo, the class can be kept as it can't hurt to identify img attachments for
> whoever might need it.
Flags: needinfo?(iann_bugzilla)
Depends on: 1381969

Comment 32

9 days ago
I would say yes
Flags: needinfo?(iann_bugzilla)
Depends on: 1382133

Comment 33

8 days ago
(In reply to Thomas D. (currently busy elsewhere; needinfo?me) from comment #31)
> Ian, does SeaMonkey want this? Nice feature, simple patch...
> Which would allow us to do the cleanup mentioned below...

That's part of bug 939540, just nobody got around yet to come up with a patch.
(In reply to rsx11m from comment #33)
> That's part of bug 939540, just nobody got around yet to come up with a
> patch.

Good catch, thanks, hadn't seen the dependency.
You need to log in before you can comment on or make changes to this bug.