780436 - double or wavy decoration lines are not repainted correctly if it's specified to floating first letter

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

At bug 59109, I forgot to fix the overflow computation of floating first letter.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=a6c48ddcdabb

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 649063 [details] [diff] [review]
Patch

I'm not sure if we check whether the frame has decoration lines actually.

If you have some idea to write automated tests for this, let me know it.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd9cd6608ca4

Comment 4

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

Does nsStyleTextReset::CalcDifference cause a reflow style hint in all the cases where the result of this calculation could change?

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fd9cd6608ca4

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to David Baron [:dbaron] from comment #4)
> Does nsStyleTextReset::CalcDifference cause a reflow style hint in all the
> cases where the result of this calculation could change?

Ah, it's good point. Currently, if the line style is changed to/from double or wavy, it returns NS_STYLE_HINT_REFLOW because solid/dotted/dashed are always same thickness. However, there is "-moz-none". I think that nsStyleTextReset::CalcDifference() should returns NS_STYLE_HINT_REFLOW for it or we should include solid underline rect to the overflow rect even if the style is none.

I like the former better because the latter is hacky. We may be able to take same mistake in the future. Do you agree?

Comment 7

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

I think in general we've been taking the opposite approach -- trying to include the underline in the overflow rect (at least for common underline styles) so that we don't have to reflow.

Then again, I think Mats was working on supporting a stronger variant of UpdateOverflowArea that might be sufficient here (in addition to repaint).

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Additional Patch

So, do you think that this is right approach?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

dbaron: ping

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 649949 [details] [diff] [review]
Additional Patch

I filed bug 812143 for this.