Bug 28707 – [Chromium] Complex text doesn't show up with text stroking

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Yusuke Sato]

Chromium can't render complex text with -webkit-text-stroke-width style.
This causes LayoutTests/fast/text/stroking-decorations.html and LayoutTests/fast/text/stroking.html tests to fail.

http://code.google.com/p/chromium/issues/detail?id=20130

[Yusuke Sato]

Created attachment 38539 [details]
complex_text_v1


---
 2 files changed, 13 insertions(+), 1 deletions(-)

[Yusuke Sato]

Created attachment 38540 [details]
complex_text_v2


---
 2 files changed, 13 insertions(+), 1 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 38540 [details]
complex_text_v2

Clearing flags on attachment: 38540

Committed r47748: <http://trac.webkit.org/changeset/47748>

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.

[Yusuke Sato]

Created attachment 38600 [details]
complex_text_second_fix_v1


---
 3 files changed, 26 insertions(+), 7 deletions(-)

[Yusuke Sato]

Uploaded second patch as per Dirk's suggestion.

[Darin Fisher (:fishd, Google)]

Comment on attachment 38600 [details]
complex_text_second_fix_v1

> [phone]  Yusuke Sato  <[e-mail]>
...
> +        Added extra NULL checks for HDC. This is the second fix for issue 28707,
> +	and the fix is similar to http://trac.webkit.org/changeset/45482.

nit: please do not include tabs in the ChangeLog.  R- for this reason.


> +++ b/WebCore/platform/graphics/chromium/UniscribeHelper.cpp

> +            if (dc) {
> +                if (firstRun) {
> +                    oldFont = SelectObject(dc, shaping.m_hfont);
> +                    firstRun = false;
> +                } else
> +                    SelectObject(dc, shaping.m_hfont);
> +            }

This is OK, but maybe it should use the useWindowsDrawing local variable
instead just as the code within the for loop does.  What do you think?

-Darin

[Yusuke Sato]

Created attachment 38601 [details]
complex_text_second_fix_v2


---
 3 files changed, 26 insertions(+), 7 deletions(-)

[Yusuke Sato]

> nit: please do not include tabs in the ChangeLog.  R- for this reason.

Fixed, sorry.

> This is OK, but maybe it should use the useWindowsDrawing local variable
> instead just as the code within the for loop does.  What do you think?

Agreed. Using useWindowsDrawing seems better than mine.
Please take another look.

--Yusuke

[Eric Seidel (no email)]

bots are red. Once they green up the commit-queue will land this.

[Yusuke Sato]

Eric,
The patch v2 seems not to be landed yet, but the commit queue looks empty.
Could you let me know hot to fix this (or could you land the patch)?

Thanks,
Yusuke

[Eric Seidel (no email)]

The bots have been red all night.  :)  This should be landing in a matter of hours now that folks are back at work.

[Shinichiro Hamaji]

As it is still not committed, I tried to remove cq+ and give cq+ again. But it seemed not to work. I don't know what we should do... Could someone check this? Thanks!

[Eric Seidel (no email)]

Oh.  this bug is closed as fixed!  Hence it's not appearing in the queue.

[WebKit Commit Bot]

Comment on attachment 38601 [details]
complex_text_second_fix_v2

Clearing flags on attachment: 38601

Committed r48313: <http://trac.webkit.org/changeset/48313>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Sato]

> this bug is closed as fixed!  Hence it's not appearing in the queue.

Ah, I get it now! Eric, Shinichiro, thanks for handling this.