Bug 34234 - [chromium] hebrew vowel marks incorrectly positioned
Summary: [chromium] hebrew vowel marks incorrectly positioned
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-27 17:04 PST by Evan Martin
Modified: 2010-02-02 00:06 PST (History)
3 users (show)

See Also:


Attachments
patch (5.59 KB, patch)
2010-01-27 17:09 PST, Evan Martin
levin: review+
levin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Evan Martin 2010-01-27 17:04:05 PST
[chromium] hebrew vowel marks incorrectly positioned
Comment 1 Evan Martin 2010-01-27 17:09:21 PST
Created attachment 47578 [details]
patch
Comment 2 Evan Martin 2010-01-27 17:11:06 PST
agl: the meat of this change is the HarfbuzzSkia change -- it turns out, confusingly, that metrics->x is the offset and metrics->xOffset is the advance.
Comment 3 Adam Langley 2010-01-28 02:53:50 PST
LGTM
Comment 4 Evan Martin 2010-01-28 08:54:42 PST
Comment on attachment 47578 [details]
patch

Note this is lacking a Mac baseline.  I will make one today, but if someone could review the code I'd appreciate it.
Comment 5 David Levin 2010-01-28 11:20:47 PST
Comment on attachment 47578 [details]
patch

Please add results before landing.
Comment 6 Eric Seidel (no email) 2010-02-01 16:15:04 PST
Evan isn't a committer (or is he?)  I assume he's supposed to post a new patch?
Comment 7 Evan Martin 2010-02-01 16:20:31 PST
This landed in r54020.  Not sure why the bug didn't close.
Comment 8 Brian Weinstein 2010-02-01 23:51:53 PST
Checked in new expected results for Windows w/ r54209. Is this expected (font issue)? Or should I file a new bug about needing different results?
Comment 9 Evan Martin 2010-02-02 00:05:11 PST
Thanks for checking, and sorry for the additional work.  I would expect this to differ slightly between Mac and Windows due to different fonts being available.  The real value of the test is as a pixel test; I tried to make the test text make testing pass/fail possible for people who can't read Hebrew.

I added this test because the code change that accompanied it fixed the test; I believe it already passed on the other WebKit ports.
Comment 10 Brian Weinstein 2010-02-02 00:06:30 PST
No problem, just wanted to make sure I wasn't masking a bug.(In reply to comment #9)
> Thanks for checking, and sorry for the additional work.  I would expect this to
> differ slightly between Mac and Windows due to different fonts being available.
>  The real value of the test is as a pixel test; I tried to make the test text
> make testing pass/fail possible for people who can't read Hebrew.
> 
> I added this test because the code change that accompanied it fixed the test; I
> believe it already passed on the other WebKit ports.

No problem, just wanted to make sure I wasn't masking over a bug and needing to file a new one. I think there is one about different fonts on Mac and Windows, which is why there are a good number of tests on the Windows Skipped list.