377364 - Page Info: Media tab should be consistent with the the Image Properties in the Element Properties window.

Status: RESOLVED FIXED

(Firefox :: Page Info Window, defect)

Description

[Florian Quèze [:florian]]

Page Info should be consistent with the element properties window on the way it displays the type of an image. See bug 344660.

Comment 1

[Dão Gottwald [:dao]]

And "Address" should be "Location", as in the Element Properties. I'm not going to file a bug for that, maybe you can fix it here. On the other hand, I could imagine that using the same dtd for both cases is the right thing to do, which would be worth a separate bug.

Comment 2

[Florian Quèze [:florian]]

We also have:

  Page Info / Media tab       | Element Properties / Image Properties
 ------------------------------------------------------------------------
  Size                        | Size of File
                              |
  Alternate Text              | Alternate text
                              |
  Specified Dimensions: N x N |
  Actual Dimensions: N x N    | Image Dimensions: N x N (scaled to N x N)

And the order is different: for example in Page Info / Media, "Alternate Text" is right under "Address", but it is the last line of the Image Properties in the Element Properties window.

Comment 3

[Florian Quèze [:florian]]

Attachment: screenshot illustrating the problem

We also have a compactness issue in the bottom part of the Media tab of Page Info. The number of rows can vary between 3 and 9.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

yup, this is right

Comment 5

[Florian Quèze [:florian]]

Attachment: patch v1

I focused more on the compactness issue than on being very consistent with the Element Properties window.

Copied from the Element Properties window:
 * The specified dimensions and actual dimensions (when different) are on the same row,
 * When the size is unknown, display "Unknown (not cached)" and get rid of the "Cache Source" row,
 * When the MIME type is image/xxx, display it as XXX Image (and display the number of frames for animated pictures).

As we are not trying to provide information on the DOM element of the img tag, but on the picture, the difference between "Title" and "Alternate Text" is irrelevant.  In the previous version of Page Info, these two rows when both set nearly always displayed the same value.  They are now merged into a single "Associated Text" row.  I know we will probably hear some screaming here but I think it's the right thing to do.

The row "Type" is now always displayed. When we can't get the MIME type, we fall back to the value of the "Type" column of the treeview.

A min-height value for the grid was added in the CSS to always keep enough room for 5 rows, to prevent the jumping behavior we had before.  The only (uncommon) case that will still make the UI jump is when both the alt (or title) and longdesc attributes are set on the HTML tag.

Mano: The patch looks bigger than it actually is: most of the "new" code is copied from metadata.js (for example, the code used to get the frame count).

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 298380 [details] [diff] [review]
patch v1

This is great!

Comment 7

[Asaf Romano (gone)]

Comment on attachment 298380 [details] [diff] [review]
patch v1

r=mano

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 298380 [details] [diff] [review]
patch v1

a=beltzner for 1.9

Comment 9

[Florian Quèze [:florian]]

Checked in.

I removed the braces around this line before checking in, per Mano's request over IRC:
sizeText = gBundle.getString("mediaUnknownNotCached");