11406 - [FONT MAC]Mac GFX doesn't handle NS_FONT_DECORATION_OVERLINE or NS_FONT_DECORATION_LINE_THROUGH

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Mike Pinkerton (not reading bugmail)]

Mac GFX doesn't handle NS_FONT_DECORATION_OVERLINE or
NS_FONT_DECORATION_LINE_THROUGH in nsFontMetricsMac.cpp. Right now they just
underline, which is not right, but i wanted to get the cases in there for
completeness.

Comment 1

[dcone (gone)]

This bug has been marked future because we have determined that it is not 
critical for netscape 6.0.  If you feel this is an error, or if it blocks your 
work in some way -- please attach your concern to the bug for reconsideration.

Comment 2

[Hixie (not reading bugmail)]

Does this bug mean that on Mac, we don't support 'text-decoration:overline' and
'text-decoration:line-through' ?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Well, for lack of a response, and lack of a Mac, I'll assume it does and 
nominate it for nsbeta3, since it's a basic css1 issue and platform parity.

Comment 4

[dcone (gone)]

This has to be disscussed,,, not just moved to Beta 3.. 

Comment 5

[Mike Pinkerton (not reading bugmail)]

this has to be discussed, not just moved to future...

Comment 6

[Pierre Saslawsky]

The decorations are handled in nsTextFrame::PaintTextDecorations(). Overline and 
line-through work correctly on the Mac, as you can see when displaying Test0.

Pink, why did you add the 4 lines with the comment "THIS IS WRONG, BUT HERE FOR 
COMPLETENESS" in nsFontMetricsMac? Maybe we can remove them and close the bug as 
Invalid.

Comment 7

[Mike Pinkerton (not reading bugmail)]

i wanted to get the cases in there so there was something to hang the correct 
code on when it was written, but underline is obviously the wrong style for 
overline and line-through, hence the comment.

Comment 8

[Pierre Saslawsky]

Got it. I confirm that this bug can be left as Future because of the way we draw 
text in Layout. We cache a font without decorations ("mNormalFont" with 
NS_FONT_DECORATION_NONE) and we draw the decorations ourselves without using GFX.

Comment 9

[Mike Pinkerton (not reading bugmail)]

if this works as pierre says (which would be a good thing), then we should remove 
the relnote2 keyword. anyone?

Comment 10

[timeless]

If it's working then removing relnote2 makes sense. I really don't understand 
what's left in this bug. Does it deserve the arch keyword?

Comment 11

[Pierre Saslawsky]

Changing keywords from "css1, fonts, nsbeta3, pp, relnote2, verifyme" to "fonts, 
pp"

Comment 12

[Steve Dagley]

Can someone please put this bug out of its misery with a FIXED, INVALID or 
WONTFIX to the head?

Comment 13

[Simon Fraser [no longer active]]

Fixing it is, of course, preferable. Don?

Comment 14

[Boris Zbarsky [:bzbarsky]]

The relevant code has moved to nsFontUtils.  But the question remains -- is this
a problem?

Comment 15

[Simon Fraser [no longer active]]

Taking

Comment 16

[Simon Fraser [no longer active]]

Attachment: Testcase (works fine on Mac)

Comment 17

[Simon Fraser [no longer active]]

Attachment: Patch to remove bogus code from nsFontUtils

Patch to remove setting underline/linethrough types in the native font style.
This is all handled by nsTextFrame:
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsTextFrame.cpp#1846

Comment 18

[Simon Montagu :smontagu]

Comment on attachment 115812 [details] [diff] [review]
Patch to remove bogus code from nsFontUtils

r=smontagu

Comment 19

[kinmoz]

Comment on attachment 115812 [details] [diff] [review]
Patch to remove bogus code from nsFontUtils

sr=[e-mail]

Comment 20

[Simon Fraser [no longer active]]

Checked in.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9eef886ab0ce
[fenix] For https://github.com/mozilla-mobile/fenix/issues/11406 - Scrolls to selected tab when switching modes