Last Comment Bug 780436 - double or wavy decoration lines are not repainted correctly if it's specified to floating first letter
: double or wavy decoration lines are not repainted correctly if it's specified...
Status: RESOLVED FIXED
: css3
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
Depends on: 812143
Blocks: 59109
  Show dependency treegraph
 
Reported: 2012-08-04 21:57 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2012-11-15 04:51 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (3.16 KB, patch)
2012-08-04 22:02 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
roc: review+
Details | Diff | Splinter Review
Additional Patch (12.36 KB, patch)
2012-08-07 20:57 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-04 21:57:04 PDT
At bug 59109, I forgot to fix the overflow computation of floating first letter.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-04 22:02:07 PDT
Created attachment 649063 [details] [diff] [review]
Patch

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=a6c48ddcdabb
Comment 2 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-04 23:56:55 PDT
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] (Mozilla Japan) 2012-08-05 17:53:28 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd9cd6608ca4
Comment 4 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-08-05 18:09:08 PDT
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] 2012-08-06 07:43:23 PDT
https://hg.mozilla.org/mozilla-central/rev/fd9cd6608ca4
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-08-06 16:38:12 PDT
(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+2 (mostly busy through August 4; review requests must explain patch) 2012-08-06 16:46:29 PDT
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] (Mozilla Japan) 2012-08-07 20:57:07 PDT
Created attachment 649949 [details] [diff] [review]
Additional Patch

So, do you think that this is right approach?
Comment 9 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-10-17 18:02:06 PDT
dbaron: ping
Comment 10 Masayuki Nakano [:masayuki] (Mozilla Japan) 2012-11-15 04:51:42 PST
Comment on attachment 649949 [details] [diff] [review]
Additional Patch

I filed bug 812143 for this.

Note You need to log in before you can comment on or make changes to this bug.