Image Properties window should have image dimensions info

VERIFIED FIXED in mozilla0.9.4

Status

SeaMonkey
UI Design
P3
enhancement
VERIFIED FIXED
17 years ago
9 years ago

People

(Reporter: Agustín Fernández, Assigned: gerv)

Tracking

(Blocks: 1 bug)

Trunk
mozilla0.9.4

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
It would be great if the image dimensions and (even more important) their size
(content-size) were on the properties dialog. I know that you can get their
dimensions from page info, but for that you must look for an image in a
(sometimes very) big list. It would be a lot easier to just right-click at them.
The content-size is another story. There is no way to get that information
without actually saving it and checking with another program. Web developers
(well, at least I) look for this information all the time.
I'd say gerv might like this. See bug 74121. As a related note, content size
will be shown in the new page info, in addition to will width and height.

Comment 2

17 years ago
-> XP Apps: GUI Features
Component: Browser-General → XP Apps: GUI Features
Summary: More stuff to the properties dialog → Image Properties window should have image dimensions info

Comment 3

17 years ago
reassign too. :)  sorry for the spam.
Assignee: asa → blakeross
QA Contact: doronr → sairuh

Comment 4

17 years ago
--> Gerv
Assignee: blake → gervase.markham
(Assignee)

Comment 5

17 years ago
Yep, sounds good to me. I can't promise when I'll get to it, though.

Gerv

Updated

17 years ago
Blocks: 89250
(Assignee)

Comment 6

17 years ago
I've done a patch for this - coming up. I've looked at the other Properties
window bugs to see if any of them are worth fixing while I'm there but IMHO none
of them are :-)

Gerv
(Assignee)

Comment 7

17 years ago
Created attachment 42334 [details] [diff] [review]
Patch v.1
(Assignee)

Comment 8

17 years ago
OK, try this on for size. Sicking, could you review?

I've also fixed the problem whereby the link underlining would continue to the
end of the row if there was something else in the column longer than the link. I
did this by putting each link in an hbox and using a spring.

Oops, hang on, I haven't done the content-size. Dunno if I can actually get that
info. Anyway, this patch is enough to be going on with.

Gerv
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.3
This won't show the physical width and height of the image if the image is 
scaled I think. Though this is better then nothing I would really like to get 
the physical width/height rather then the displayed. (this was actually the 
reason I didn't add width/height originally)
(Assignee)

Comment 10

17 years ago
I doubt there's any easy way of getting the physical size of the image. First
we'd have to be able to convert the handle we have on the <image> tag to a
handle to something representing the image itself, which we can ask the image
library about. That is definitely a separate, more complex RFE.

Gerv
yeah... but that is probobly where you'll find the physical size though. I 
think Daniel Brooks does something similar in bug 52730, would you mind taking 
a look at that?

Also, could you add the " pixels" string in the xul file instead by doing <text 
value=" &pixels;">. Otherwise you'll get "Width:  pixels" if the img dosn't 
have a .width property (for instance a <input type="image">)
(Assignee)

Comment 12

17 years ago
I have the latest version of db48x's patch for that bug and, as far as I can
see, there's nothing in the JS about getting widths and heights.

I think logical width and height are a good start.

Adding &pixels.label; in the XUL file will mean it gets overwritten when I set
the label with the width/height value, won't it? Is it really possible to have
an image which is visible on screen (so it can be right-clicked) which doesn't
have a width and a height? If so, I can just do:
if (width in img)
   // set width

Gerv

Gerv
Gerv: Remember that the 'img' object isn't necessarily an <img>. I'm not sure 
is a <input type="image"> has a .width property?

The " pixels" text won't be overwritten if you add it in a separate <text>. IE 
if you do:

 <row id="image-width">
   <text value="&image-width.label; "/>
   <hbox>
     <text id="image-width-text" value=""/>
     <text value=" &pixels.lable;"/>
   </hbox>
 </row>

Daniel: You say that your new page-info window will contain content-size, 
how/where do you get that?
(Assignee)

Comment 14

17 years ago
Created attachment 43214 [details] [diff] [review]
Patch v.2
(Assignee)

Comment 15

17 years ago
Unless someone reviews this quick, it's not going to make it in...

Gerv
Target Milestone: mozilla0.9.3 → mozilla0.9.4
For resons not told here my ISP has cut me off so I can't really apply and test 
this patch. However it's a fairly simple patch, and assuming you have tested it 
and it works (with <input type="image"> too):

r=sicking
cc'ing db48x for input on the content-size thing. Please see my comment 2001-07-
18 07:42
(Assignee)

Comment 18

17 years ago
Created attachment 43350 [details] [diff] [review]
Remove parentheses; fix a couple of JS warnings
please move the |var i| declaration out of the first for-loop to clarify that 
the variable is used further down. Other then that, r=sicking still applies

Comment 20

17 years ago
+        if ("width" in img) {
+            setInfo("image-width", img.width);
+            setInfo("image-height", img.height);
+        }


Why is that check sufficient to assume that there's a height?

Almost nothing should use onclick.  Those <text/> don't seem to be an exception.
 Does oncommand work?
(Assignee)

Comment 21

17 years ago
> Why is that check sufficient to assume that there's a height?

It's not. But if you can think of any reason that a clickable-on image could
have a width but not a height, I'd be very impressed.

> Almost nothing should use onclick.  Those <text/> don't seem to be an 
> exception. Does oncommand work?

A quick search-and-replace yields the answer: No. :-| Can we avoid letting this
patch spread from the issue? I don't want to open other cans of worms...

Gerv
DOH! I missed that |if|. You don't need it at all, just do:

+        setInfo("image-width", img.width);
+        setInfo("image-height", img.height);

If width and/or height are undefined they will be hidden (the setInfo function 
does that) so in fact you *shouldn't* have that if
(Assignee)

Comment 23

17 years ago
No, you need the if to avoid JS strict errors.

Gerv
hmm.. didn't know that. Thought that was legal. Ok, in that case you need to add

+        else {
+            setInfo("image-width", "");
+            setInfo("image-height", "");
+        }

for the case when .width dosn't exist
(Assignee)

Comment 25

16 years ago
Done. Let's put this to bed. sicking - review, please? :-)

Gerv
(Assignee)

Comment 26

16 years ago
Created attachment 45933 [details] [diff] [review]
Patch v.4
That |else { ... }| was all you changed, right?

If so, r=sicking

Comment 28

16 years ago
sr=blake
(Assignee)

Comment 29

16 years ago
Checked in.

Gerv
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
vrfy fixed. tested at http://wired.com/news/nc_index.html, and the Image
Properties in Page Info had columns and listed data for width, height and alt text.

linux, 2001.08.15.14-comm
winnt, 2001.08.15.06-comm
mac OS 9.1 [emul on X], 2001.08.15.08-comm
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite

Updated

9 years ago
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.