WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
35495
[chromium] inverted logic in per-strike renderer patch
https://bugs.webkit.org/show_bug.cgi?id=35495
Summary
[chromium] inverted logic in per-strike renderer patch
Evan Martin
Reported
2010-02-28 09:10:50 PST
[chromium] inverted logic in per-strike renderer patch
Attachments
Patch
(1.29 KB, patch)
2010-02-28 09:11 PST
,
Evan Martin
levin
: review-
Details
Formatted Diff
Diff
patch
(2.76 KB, patch)
2010-03-08 08:17 PST
,
Adam Langley
fishd
: review-
Details
Formatted Diff
Diff
patch
(2.81 KB, patch)
2010-03-11 08:26 PST
,
Adam Langley
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Evan Martin
Comment 1
2010-02-28 09:11:34 PST
Created
attachment 49699
[details]
Patch
Darin Fisher (:fishd, Google)
Comment 2
2010-03-01 11:38:57 PST
I'd like Adam Langley to review this patch. If he's happy, then LGTM.
Darin Fisher (:fishd, Google)
Comment 3
2010-03-01 11:39:26 PST
Also, is there a way to write a layout test for this?
David Levin
Comment 4
2010-03-01 15:31:05 PST
Comment on
attachment 49699
[details]
Patch Needs a layout test, or explanation of why a layout test isn't needed or isn't possible.
Adam Langley
Comment 5
2010-03-02 10:26:45 PST
Am working on Chrome side changes to enable a layout test of this change.
Adam Langley
Comment 6
2010-03-08 08:17:13 PST
Created
attachment 50223
[details]
patch
Darin Fisher (:fishd, Google)
Comment 7
2010-03-09 16:24:51 PST
Comment on
attachment 50223
[details]
patch
> +++ b/LayoutTests/fast/text/chromium-linux-fontconfig-renderstyle.html
shouldn't there be some expected results for this test for other platforms? or should the test be skipped on other platforms? if not, then perhaps the test should not be named "chromium-linux-*"?
Adam Langley
Comment 8
2010-03-10 07:35:07 PST
(In reply to
comment #7
)
> shouldn't there be some expected results for this test for other platforms? > or should the test be skipped on other platforms? if not, then perhaps the > test should not be named "chromium-linux-*"?
The Chrome special layout tests seem to have disappeared, and I obviously can't include expected results until the patch is committed. If there's a test-expectations.txt like way to tell the webkit.org builders not to run a given test I'm afraid I don't know it. (I guess I mostly don't understand your comment, sorry!)
Evan Martin
Comment 9
2010-03-10 08:27:31 PST
It's suboptimal, but there it is: $ git ls-files | grep Skipped LayoutTests/platform/gtk/Skipped LayoutTests/platform/mac-leopard/Skipped LayoutTests/platform/mac-snowleopard/Skipped LayoutTests/platform/mac-tiger/Skipped LayoutTests/platform/mac/Skipped LayoutTests/platform/qt-linux/Skipped LayoutTests/platform/qt-mac/Skipped LayoutTests/platform/qt-win/Skipped LayoutTests/platform/qt/Skipped LayoutTests/platform/win/Skipped I'm not sure how a port-specific layout test is supposed to work.
Tony Chang
Comment 10
2010-03-10 17:04:25 PST
It looks like LayoutTests/platform/chromium already exists. Can you move the test there and put the expected png and txt file there? It should then be automatically skipped by the other webkit ports.
Adam Langley
Comment 11
2010-03-11 08:26:18 PST
Created
attachment 50506
[details]
patch Have moved the test into platform/chromium. I haven't included any expected results partly because --new-baseline is broken (that's a different issue) but mostly because it doesn't make any sense: the results will be different for each platform, so we can't put one set in platform/chromium. Also, our baselines are still carried in our Chromium tree I think.
WebKit Commit Bot
Comment 12
2010-03-18 08:41:57 PDT
Comment on
attachment 50506
[details]
patch Clearing flags on attachment: 50506 Committed
r56162
: <
http://trac.webkit.org/changeset/56162
>
WebKit Commit Bot
Comment 13
2010-03-18 08:42:02 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug