Closed Bug 774495 Opened 9 years ago Closed 9 years ago

Matches containing ligatures in awesomebar results should not be underlined, use normal style instead

Categories

(Firefox :: Address Bar, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
Firefox 17
Tracking Status
firefox16 --- fixed

People

(Reporter: ahurle, Assigned: ahurle)

References

Details

(Keywords: regression)

Attachments

(2 files)

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.
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+
(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
https://hg.mozilla.org/mozilla-central/rev/955bb9f6169c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
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?
Attachment #642853 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.
(whoops, Gavin already grabbed this)
Keywords: regression
You need to log in before you can comment on or make changes to this bug.