Closed
Bug 195481
Opened 22 years ago
Closed 21 years ago
Automatic image resize: Image status should be reflected in document title bar and Tab marker
Categories
(Core :: Layout: Images, Video, and HTML Frames, defect, P3)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
VERIFIED
FIXED
mozilla1.4beta
People
(Reporter: chrispetersen, Assigned: janv)
Details
(Whiteboard: [adt2] [UI])
Attachments
(1 file, 3 obsolete files)
7.34 KB,
patch
|
jag+mozilla
:
superreview+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•22 years ago
|
Component: Layout → Image: Layout
Reporter | ||
Updated•22 years ago
|
QA Contact: ian → petersen
Reporter | ||
Updated•22 years ago
|
Whiteboard: [adt2]
Assignee | ||
Updated•21 years ago
|
Priority: -- → P3
Target Milestone: --- → mozilla1.4beta
Assignee | ||
Updated•21 years ago
|
Whiteboard: [adt2] → [adt2] [UI]
Assignee | ||
Comment 2•21 years ago
|
||
Assignee | ||
Comment 3•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #121111 -
Flags: superreview?(jaggernaut)
Attachment #121111 -
Flags: review?(bzbarsky)
Comment 4•21 years ago
|
||
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)?
Assignee | ||
Comment 5•21 years ago
|
||
made the title separator localizable and tested on windows and linux (gcc 3.2.2 and gcc 2.95.3)
Attachment #121111 -
Attachment is obsolete: true
Attachment #121112 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #121111 -
Flags: superreview?(jaggernaut)
Attachment #121111 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•21 years ago
|
Attachment #121134 -
Flags: superreview?(jaggernaut)
Attachment #121134 -
Flags: review?(bzbarsky)
Comment 6•21 years ago
|
||
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.
Assignee | ||
Comment 7•21 years ago
|
||
> 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.
Status: NEW → ASSIGNED
Comment 8•21 years ago
|
||
> 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.
Assignee | ||
Comment 9•21 years ago
|
||
>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•21 years ago
|
||
>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...)
Assignee | ||
Comment 11•21 years ago
|
||
yes, \u0020 gets trimmed (using a trunk build)
Comment 12•21 years ago
|
||
Comment on attachment 121134 [details] [diff] [review] new patch Sigh. Could we smack alecf, please?
Attachment #121134 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 13•21 years ago
|
||
Alec, can you provide a better solution or you're ok with this approach?
Comment 14•21 years ago
|
||
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•21 years ago
|
||
I think I'd prefer "%S - %S" (title, status) and then use that if !aStatus.IsEmpty(), otherwise just use title.
Assignee | ||
Comment 16•21 years ago
|
||
Attachment #121134 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #121134 -
Flags: superreview?(jaggernaut)
Assignee | ||
Updated•21 years ago
|
Attachment #121203 -
Flags: superreview?(jaggernaut)
Comment 17•21 years ago
|
||
Comment on attachment 121203 [details] [diff] [review] revised patch sr=jag
Attachment #121203 -
Flags: superreview?(jaggernaut) → superreview+
Assignee | ||
Comment 18•21 years ago
|
||
checked in
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 19•21 years ago
|
||
Verified in the 2003-04-22-08 Macho and Win32 trunk builds.
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•