|Summary:||svg background color on selected text goes wrong when text has gradient stroke.|
|Product:||WebKit||Reporter:||MORITA Hajime <morrita>|
|Severity:||Normal||CC:||commit-queue, hamaji, webkit.review.bot|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.6|
Description MORITA Hajime 2009-12-30 19:13:32 PST
Created attachment 45695 [details] xhtml to reproduce When text has gradient stroke e.g. "stroke: url(#gradient); stroke-width: 1pt;", and these text are selected, its background unexpectedly painted with the gradient. svn revision: 52638 How to reproduce: open attached XHTML file. Expected result: All test has blue background. Actual Result: second line of text has gradient background.
Comment 2 WebKit Review Bot 2009-12-30 21:32:41 PST
Attachment 45697 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/rendering/SVGInlineTextBox.cpp:379: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators]  Total errors found: 1
Comment 3 MORITA Hajime 2010-01-02 07:59:57 PST
Created attachment 45739 [details] fix guideline violation
Comment 4 WebKit Review Bot 2010-01-02 08:05:08 PST
style-queue ran check-webkit-style on attachment 45739 [details] without any errors.
Comment 5 Eric Seidel (no email) 2010-01-02 12:15:35 PST
Comment on attachment 45739 [details] fix guideline violation In general this seems like a good idea to me. I would like to know more about why the various layout tests changed. It's not clear to me from the diffs why each layout test changed. Do they all use gradient fill text?
Comment 6 MORITA Hajime 2010-01-02 18:23:00 PST
Thank you for quick feedback! (In reply to comment #5) > Do they all use gradient fill text? No. This change affects SVG text selection rendering even if it is not with gradient. Original code paints background twice; Once at fill subphase and another at stroke subphase, in following order: 1. paintCharacters() for fill 1.1. paint background 1.2. paint glyph for fill 2. paintCharacters() for stroke 2.1. paint background (again) 2.2. paint glyph for stroke. So 2.1 (background painting on stroke subphase) overwrites the filled glyphs (with half opacity). The patch split background painting out to its own subphase, then filled glyphs are no longer overwritten. This change requires some test results rebasing. But it seems reasonable for me. Slightly OT: Painting foreground and text decorations may have same issue. But currently I have no idea to unveil them.
Comment 7 Nikolas Zimmermann 2010-01-06 18:57:00 PST
Comment on attachment 45739 [details] fix guideline violation Looks good to me, I've checked all results and they're just fine now. Glad you've started this work - do you plan to work more on SVG Text? We have some important TODO items in that area - I'd be glad if you're interessted :-)
Comment 8 MORITA Hajime 2010-01-06 22:06:57 PST
Thank you for reviewing! (In reply to comment #7) > (From update of attachment 45739 [details]) > We have some important TODO items in that area - I'd be glad if you're > interessted :-) Well, I have certain interest and will go back to Bug 15997 next. And any further suggestions and ideas to go are appreciated.
Comment 9 Eric Seidel (no email) 2010-01-06 23:45:29 PST
Comment on attachment 45739 [details] fix guideline violation We'll see if the commit-queue can stomach this monster patch.
Comment 10 WebKit Commit Bot 2010-01-07 00:07:32 PST
Comment on attachment 45739 [details] fix guideline violation Clearing flags on attachment: 45739 Committed r52904: <http://trac.webkit.org/changeset/52904>
Comment 11 WebKit Commit Bot 2010-01-07 00:07:41 PST
All reviewed patches have been landed. Closing bug.