Closed Bug 471838 Opened 13 years ago Closed 12 years ago

Help->About "Licensing information" text is wrapped after the opening parenthesis

Categories

(Firefox :: General, defect)

defect
Not set
trivial

Tracking

()

RESOLVED INVALID

People

(Reporter: Gavin, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: polish, Whiteboard: [polish-easy] [polish-visual][polish-p2])

Attachments

(3 files, 2 obsolete files)

Wrapping after the opening parenthesis is unfortunate. It'd be nicer if the entire line would be kept together on it's own line. We can probably force that by adding an extra space or two before it.
Attached image screenshot
Ugh, that's ugly. :(
Flags: wanted-firefox3.1?
Whiteboard: [polish-easy] [polish-visual]
Do we really need parentheses here? I'd say we should remove them.
That sounds reasonable, but we're past the l10n freeze. We might be able to get away with an automated change to all locales, though, since this isn't something that varies much, I think.
Why do we line break immediately after a parenthesis anyway? Is there a bogus space in the source that there shouldn't be?

Gerv
<checks> No, no bogus space. It's a bug in our text-wrapping algorithm (IMO, anyway). 

It may well only occur in English, and maybe only on Mac OS, because you'd need text of the right length (and therefore in a certain font). We could remove the parentheses in English only. :-)

Gerv
Occurs on Linux, too.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090114 Minefield/3.2a1pre ID:20090114020416
bz points out that this happens because the inner text is in a separate <label> (which is a child of the <description> that holds the wrapping lines with parentheses), and that that makes this a duplicate of a WONTFIX bug (which I can't find). We should just work around it.
The bug I was thinking of is bug 472126, of which this looks like a duplicate.
Well, we can fix this one without fixing that one (by using a workaround in the dialog XUL), so not quite duplicates I would say.
Depends on: 472126
Duplicate of this bug: 475287
Attached patch Ugly hack to get around this. (obsolete) — Splinter Review
Just posting it here before I drop it, this was the only way I could get the "(" to show up on the next line (aside from making l10n changes).
Assignee: nobody → dao
Attached patch patch (obsolete) — Splinter Review
Attachment #368816 - Flags: review?(gavin.sharp)
Comment on attachment 368816 [details] [diff] [review]
patch

>diff --git a/browser/base/content/aboutDialog.css b/browser/base/content/aboutDialog.css

>+html|a:focus {
>+  outline: 1px dotted;

Why outline instead of border? Should you specify -moz-DialogText as the toolkit themes do for their border?
What about the color changes (red for focus)?

This seems to break keyboard access (can't tab to the link and press enter). Strangely enough, it seems to work if I use Ctrl+Enter (!?).
(In reply to comment #14)
> Why outline instead of border? Should you specify -moz-DialogText as the
> toolkit themes do for their border?
> What about the color changes (red for focus)?

Outline is the standard tool for drawing focus rings.
The text-link styling in global.css seems arbitrary... Red for :focus doesn't make much sense. In HTML it's usually just red for :active [1][2], but even that wouldn't be right for native apps on Windows.
I could copy the styling from global.css, but it doesn't make much sense to me.

[1]: http://hg.mozilla.org/mozilla-central/annotate/526a987a3b1e/layout/style/ua.css#l114
[2]: http://hg.mozilla.org/mozilla-central/annotate/526a987a3b1e/layout/base/nsPresShell.cpp#l2158
Comment on attachment 368816 [details] [diff] [review]
patch

Need to fix the keyboard access
Attachment #368816 - Attachment is obsolete: true
Attachment #368816 - Flags: review?(gavin.sharp)
Ok, do we have bugs on file on fixing the toolkit .text-link styling?
Attached patch patch v2Splinter Review
Attachment #369134 - Flags: review?(gavin.sharp)
(In reply to comment #17)
> Ok, do we have bugs on file on fixing the toolkit .text-link styling?

I don't think so, I'll file one.
Comment on attachment 369134 [details] [diff] [review]
patch v2

>diff --git a/toolkit/content/widgets/text.xml b/toolkit/content/widgets/text.xml

>-      <handler event="keypress" preventdefault="true" keycode="VK_ENTER"  action="this.click()" />
>-      <handler event="keypress" preventdefault="true" keycode="VK_RETURN" action="this.click()" />
>+      <handler event="keypress" keycode="VK_ENTER"  action="this._handleEnter(event);"/>
>+      <handler event="keypress" keycode="VK_RETURN" action="this._handleEnter(event);"/>

This preventdefault removal seems to be sufficient to fix keyboard access... why the other changes?
This gives you "Error: this.click is not a function" in the error console, and we'd bypass the security check. I'm also not sure that not doing preventDefault for the keypress event would be a good idea for text-link labels.
Oh... why don't we just call open() directly in both cases? Not sure I see what calling click() gets us.

What default action are we preventing in the label case? I don't see any negative side effects in the engine manager with preventdefault="true" removed (no system beep, etc.).
(In reply to comment #22)
> Oh... why don't we just call open() directly in both cases? Not sure I see what
> calling click() gets us.

That would be ideal, but it would also break the engine manager (and others), as it uses onclick but no href attribute.

> What default action are we preventing in the label case? I don't see any
> negative side effects in the engine manager with preventdefault="true" removed
> (no system beep, etc.).

The label could be inside of some thing that consumes the event depending on getPreventDefault().
(In reply to comment #23)
> (In reply to comment #22)
> > Oh... why don't we just call open() directly in both cases? Not sure I see what
> > calling click() gets us.
> 
> That would be ideal, but it would also break the engine manager (and others),
> as it uses onclick but no href attribute.

likely because of bug 263433
Flags: wanted-firefox3.6?
This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.

Unlike the focus ring problem, this is more clearly broken.
Whiteboard: [polish-easy] [polish-visual] → [polish-easy] [polish-visual][polish-p2]
Duplicate of this bug: 501301
Confirmed in FF 3.5 final en_US.
Attached image FF3.5final
Flags: wanted-firefox3.5?
Attachment #364734 - Attachment is obsolete: true
Flags: wanted-firefox3.6?
This bug is no longer valid now that the patch in bug 536336 has landed.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
Assignee: dao → nobody
Attachment #369134 - Flags: review?(gavin.sharp)
Depends on: 536336
Duplicate of this bug: 558634
You need to log in before you can comment on or make changes to this bug.