Closed
Bug 774495
Opened 12 years ago
Closed 12 years ago
Matches containing ligatures in awesomebar results should not be underlined, use normal style instead
Categories
(Firefox :: Address Bar, defect)
Firefox
Address Bar
Tracking
()
RESOLVED
FIXED
Firefox 17
Tracking | Status | |
---|---|---|
firefox16 | --- | fixed |
People
(Reporter: ahurle, Assigned: ahurle)
References
Details
(Keywords: regression)
Attachments
(2 files)
5.76 KB,
image/png
|
Details | |
5.09 KB,
patch
|
Unfocused
:
review+
Gavin
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
In bug 407861, an underline was introduced for matches which had ligatures since bolding the text would break ligatures. Now that we're using a highlighting style without bold text, all matches should use that new style since it shouldn't break anything. Should be able to just get rid of the logic that checks for alt emphasis, and use ac-emphasize-text on all matches instead.
Assignee | ||
Comment 1•12 years ago
|
||
Try push: https://tbpl.mozilla.org/?tree=Try&noignore=1&rev=a6bef3014f3a https://tbpl.mozilla.org/?tree=Try&noignore=1&rev=f9cd3dda095b
Attachment #642853 -
Flags: review?(bmcbride)
Comment 2•12 years ago
|
||
Comment on attachment 642853 [details] [diff] [review] v1 Remove _needsAlternateEmphasis and alt styling Review of attachment 642853 [details] [diff] [review]: ----------------------------------------------------------------- Ever so vaguely concerned about other apps needing that class, but I think the likely hood of them using bold but not using the built-in styles is very slim. Also, less untested code = win.
Attachment #642853 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Blair McBride (:Unfocused) from comment #2) > Comment on attachment 642853 [details] [diff] [review] > v1 Remove _needsAlternateEmphasis and alt styling > > Review of attachment 642853 [details] [diff] [review]: > ----------------------------------------------------------------- > > Ever so vaguely concerned about other apps needing that class, but I think > the likely hood of them using bold but not using the built-in styles is very > slim. Also, less untested code = win. I had the same thought process :) Thought about leaving the alt class there just in case it was needed, but that makes things messier with little (if any) benefit.
Keywords: checkin-needed
Comment 4•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/955bb9f6169c Try run is in comment 1.
Flags: in-testsuite-
Keywords: checkin-needed
Updated•12 years ago
|
status-firefox16:
--- → affected
Comment 5•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/955bb9f6169c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 642853 [details] [diff] [review] v1 Remove _needsAlternateEmphasis and alt styling [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 587909, bug 407861 User impact if declined: Minor - users encountering affected languages will needlessly experience inconsistent highlight styles in results until Firefox 17 Testing completed (on m-c, etc.): Yes, see try/m-i/m-c pushes above Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: None
Attachment #642853 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Attachment #642853 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 7•12 years ago
|
||
Comment on attachment 642853 [details] [diff] [review] v1 Remove _needsAlternateEmphasis and alt styling [Triage Comment] Fix for a regression in FN16, approved for Aurora 16 since we're still early in the cycle.
Comment 9•12 years ago
|
||
Thanks. https://hg.mozilla.org/releases/mozilla-aurora/rev/58b2e6a5eae8
You need to log in
before you can comment on or make changes to this bug.
Description
•