Closed
Bug 534083
Opened 15 years ago
Closed 11 years ago
Provide image scaling for embedded images
Categories
(Thunderbird :: Mail Window Front End, enhancement)
Thunderbird
Mail Window Front End
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 28.0
People
(Reporter: dale223223, Assigned: alta88)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
(Whiteboard: [tb31features])
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
alta88
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•15 years ago
|
||
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 ?
Updated•15 years ago
|
Component: General → Mail Window Front End
QA Contact: general → front-end
No error messages, still happening in safe mode. I am sending one that is doing it right now to: ludovic@mozillamessaging.com.
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
Comment 5•15 years ago
|
||
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"><judy@courierjournal.net></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"><artdepartment@courierjournal.net></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 <<a moz-do-not-send="true"
href="mailto:willowsinfo@gmail.com">willowsinfo@gmail.com</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>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•15 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.
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•15 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
Comment 13•13 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?
Updated•11 years ago
|
Updated•11 years ago
|
Assignee | ||
Comment 14•11 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)
Comment 15•11 years ago
|
||
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•11 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•11 years ago
|
||
Assignee: nobody → alta88
Attachment #818013 -
Flags: review?(bwinton)
Assignee | ||
Comment 18•11 years ago
|
||
some tweaks.
Attachment #818013 -
Attachment is obsolete: true
Attachment #818013 -
Flags: review?(bwinton)
Attachment #818107 -
Flags: review?(bwinton)
Assignee | ||
Comment 19•11 years ago
|
||
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•11 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•11 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•11 years ago
|
||
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•11 years ago
|
||
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)
Assignee | ||
Comment 24•11 years ago
|
||
ping?
Comment 25•11 years ago
|
||
(Sorry, new computer can't build Thunderbird yet! I'll try to get to it on the old computer this weekend!)
Comment 26•11 years ago
|
||
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•11 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•11 years ago
|
||
updated for comments.
Attachment #820045 -
Attachment is obsolete: true
Attachment #830210 -
Flags: review+
Keywords: checkin-needed
Assignee | ||
Comment 29•11 years ago
|
||
tiny tweak to handle broken images.
Attachment #830210 -
Attachment is obsolete: true
Attachment #830240 -
Flags: review+
Comment 30•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 28.0
Updated•7 years ago
|
Comment 31•7 years ago
|
||
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)
Comment 33•7 years 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.
Comment 34•7 years ago
|
||
(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.
Description
•