Bug 28707

Summary: [Chromium] Complex text doesn't show up with text stroking
Product: WebKit Reporter: Yusuke Sato <yusukes>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric, hamaji, ukai
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows Vista   
Attachments:
Description Flags
complex_text_v1
none
complex_text_v2
none
complex_text_second_fix_v1
fishd: review-
complex_text_second_fix_v2 none

Description Yusuke Sato 2009-08-25 03:41:39 PDT
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
Comment 1 Yusuke Sato 2009-08-25 04:05:09 PDT
Created attachment 38539 [details]
complex_text_v1


---
 2 files changed, 13 insertions(+), 1 deletions(-)
Comment 2 Yusuke Sato 2009-08-25 04:17:55 PDT
Created attachment 38540 [details]
complex_text_v2


---
 2 files changed, 13 insertions(+), 1 deletions(-)
Comment 3 Eric Seidel (no email) 2009-08-25 09:29:00 PDT
Comment on attachment 38540 [details]
complex_text_v2

Clearing flags on attachment: 38540

Committed r47748: <http://trac.webkit.org/changeset/47748>
Comment 4 Eric Seidel (no email) 2009-08-25 09:29:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Yusuke Sato 2009-08-25 23:26:11 PDT
Created attachment 38600 [details]
complex_text_second_fix_v1


---
 3 files changed, 26 insertions(+), 7 deletions(-)
Comment 6 Yusuke Sato 2009-08-25 23:27:41 PDT
Uploaded second patch as per Dirk's suggestion.
Comment 7 Darin Fisher (:fishd, Google) 2009-08-25 23:40:00 PDT
Comment on attachment 38600 [details]
complex_text_second_fix_v1

> +2009-08-25  Yusuke Sato  <yusukes@chromium.org>
...
> +        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
Comment 8 Yusuke Sato 2009-08-26 00:11:03 PDT
Created attachment 38601 [details]
complex_text_second_fix_v2


---
 3 files changed, 26 insertions(+), 7 deletions(-)
Comment 9 Yusuke Sato 2009-08-26 00:12:28 PDT
> 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
Comment 10 Eric Seidel (no email) 2009-08-26 03:11:37 PDT
bots are red. Once they green up the commit-queue will land this.
Comment 11 Yusuke Sato 2009-08-27 21:25:49 PDT
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
Comment 12 Eric Seidel (no email) 2009-09-11 09:29:25 PDT
The bots have been red all night.  :)  This should be landing in a matter of hours now that folks are back at work.
Comment 13 Shinichiro Hamaji 2009-09-11 11:57:57 PDT
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!
Comment 14 Eric Seidel (no email) 2009-09-11 12:59:27 PDT
Oh.  this bug is closed as fixed!  Hence it's not appearing in the queue.
Comment 15 WebKit Commit Bot 2009-09-11 13:09:20 PDT
Comment on attachment 38601 [details]
complex_text_second_fix_v2

Clearing flags on attachment: 38601

Committed r48313: <http://trac.webkit.org/changeset/48313>
Comment 16 WebKit Commit Bot 2009-09-11 13:09:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Yusuke Sato 2009-09-11 17:28:45 PDT
> 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.