195481 - Automatic image resize: Image status should be reflected in document title bar and Tab marker

Status: VERIFIED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

Description

[Chris Petersen]

When automatic image resize is enabled, we need to indicate the image's current
status. One idea is too have the status name (Normal or Shrink to Fit)  to be
displayed in the documents tab or window title bar.

Comment 1

[Jan Varga [:janv]]

-> me

Comment 2

[Jan Varga [:janv]]

Attachment: patch

Comment 3

[Jan Varga [:janv]]

Attachment: diff -w version

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 121111 [details] [diff] [review]
patch

>+    SetTitle(title + NS_LITERAL_STRING(" - ") + aStatus);

Imo, things like that dash should really be localized, not hardcoded.

>+                             const nsAString&   aStatus = NS_LITERAL_STRING(""));

You've tested that on both Windows and Linux (the latter without
--fshort-wchar)?

Comment 5

[Jan Varga [:janv]]

Attachment: new patch

made the title separator localizable and tested on windows and linux (gcc 3.2.2
and gcc 2.95.3)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 121134 [details] [diff] [review]
new patch

Um... Why \u00a0 instead of regular spaces?

Also, it's usually better to localize the whole string so that localizers can
rearrange it as needed to fit the conventions of the target language, rather
than localizing the pieces and concatenating them in a hardcoded way.

Comment 7

[Jan Varga [:janv]]

> Um... Why \u00a0 instead of regular spaces?
There's no other way to do it, the strings are trimmed during parsing.
00a0 is no-break space according to http://www.unicode.org/charts/PDF/U0000.pdf
That's exactly what we need.

Btw, nsContentTreeOwner::SetTitle() does similar thing, it uses standalone
mTitleSeparator too.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> 00a0 is no-break space

Right.  Why not use the unicode escape for a normal vanilla breaking space? 
That should have much better chance of being supported in titlebars...

And yes, I know that nsContentTreeOwner does that.  I feel that that's wrong too...

Apart from these issues, r=me; in the end I really don't care that much about
any of this stuff as long as it does not adversely affect gecko proper.

Comment 9

[Jan Varga [:janv]]

>Right.  Why not use the unicode escape for a normal vanilla breaking space? 
>That should have much better chance of being supported in titlebars...

The problem here is that if I use standard space it will be trimmed during parsing.

Thanks for the review.

Comment 10

[Boris Zbarsky [:bzbarsky]]

>The problem here is that if I use standard space it will be trimmed during
>parsing.

It will?

|+TitleSeparator= - |

Would get trimmed.

|+TitleSeparator=\u0020-\u0020|

should not... if it does, that's a bug (not that I'd be too surprised, come to
think of it...)

Comment 11

[Jan Varga [:janv]]

yes, \u0020 gets trimmed (using a trunk build)

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 121134 [details] [diff] [review]
new patch

Sigh.  Could we smack alecf, please?

Comment 13

[Jan Varga [:janv]]

Alec, can you provide a better solution or you're ok with this approach?

Comment 14

[Alec Flett]

don't smack me! :)
I didn't write that string-trimming stuff (I'm cvs blamed only because I backed
out my parser rewrite some time ago)

I think that its part of the .properties file spec which came from java. (I'm
not defending it, just explaining it)

Comment 15

[jag (Peter Annema)]

I think I'd prefer "%S - %S" (title, status) and then use that if
!aStatus.IsEmpty(), otherwise just use title.

Comment 16

[Jan Varga [:janv]]

Attachment: revised patch

Comment 17

[jag (Peter Annema)]

Comment on attachment 121203 [details] [diff] [review]
revised patch

sr=jag

Comment 18

[Jan Varga [:janv]]

checked in

Comment 19

[Chris Petersen]

Verified in the 2003-04-22-08 Macho and Win32 trunk builds.