Bug 67723 - Fixed Layout Mode should be adjustable from layoutTestController for testing on Chromium platforms
Summary: Fixed Layout Mode should be adjustable from layoutTestController for testing ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fady Samuel
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-07 11:47 PDT by Fady Samuel
Modified: 2011-09-08 12:19 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.38 KB, patch)
2011-09-07 11:47 PDT, Fady Samuel
no flags Details | Formatted Diff | Diff
Patch (8.69 KB, patch)
2011-09-07 14:20 PDT, Fady Samuel
no flags Details | Formatted Diff | Diff
Wrong bug: please ignore. (237.01 KB, image/png)
2011-09-07 14:47 PDT, Fady Samuel
no flags Details
Patch (7.49 KB, patch)
2011-09-07 15:44 PDT, Fady Samuel
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fady Samuel 2011-09-07 11:47:04 PDT
Fixed Layout Mode should be adjustable from eventSender for testing on Chromium platforms
Comment 1 Fady Samuel 2011-09-07 11:47:53 PDT
Created attachment 106614 [details]
Patch
Comment 2 Adam Barth 2011-09-07 14:00:54 PDT
This patch looks good, but the API should probably be on layoutTestController.  EventSender is really more for sending events.
Comment 3 Fady Samuel 2011-09-07 14:20:48 PDT
Created attachment 106637 [details]
Patch
Comment 4 Adam Barth 2011-09-07 14:24:36 PDT
Comment on attachment 106637 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=106637&action=review

Why do you have two copies of fixed-layout-360x240-expected.png ?  They have the same sha1.  Presumably we can remove LayoutTests/platform/chromium-linux/fast/repaint/fixed-layout-360x240-expected.png.

> LayoutTests/fast/repaint/fixed-layout-360x240.html:8
> +            window.layoutTestController.enableFixedLayoutMode(true);
> +            window.layoutTestController.setFixedLayoutSize(360, 240);

Does this test need to be skipped on non-chromium platforms?
Comment 5 Fady Samuel 2011-09-07 14:47:34 PDT
Created attachment 106642 [details]
Wrong bug: please ignore.
Comment 6 Fady Samuel 2011-09-07 14:51:38 PDT
(In reply to comment #4)
> (From update of attachment 106637 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=106637&action=review
> 
> Why do you have two copies of fixed-layout-360x240-expected.png ?  They have the same sha1.  Presumably we can remove LayoutTests/platform/chromium-linux/fast/repaint/fixed-layout-360x240-expected.png.
> 
> > LayoutTests/fast/repaint/fixed-layout-360x240.html:8
> > +            window.layoutTestController.enableFixedLayoutMode(true);
> > +            window.layoutTestController.setFixedLayoutSize(360, 240);
> 
> Does this test need to be skipped on non-chromium platforms?

It does, I'm not sure how to do that? Do I just include a different layout test result as the default and another under the chromium directory?
Comment 7 Adam Barth 2011-09-07 14:54:08 PDT
> It does, I'm not sure how to do that? Do I just include a different layout test result as the default and another under the chromium directory?

In this case, because this test is testing a Chromium WebKit API, I would put the test in the LayoutTest/platform/chromium/fast/repaint/fixed-layout-360x240.html

That will cause the test to only be run on Chromium.  Another approach is to add the test to all the other port's Skipped files, which would be appropriate if this was a test for a feature we'd expect other ports to implement eventually.
Comment 8 Fady Samuel 2011-09-07 15:44:09 PDT
Created attachment 106662 [details]
Patch
Comment 9 Fady Samuel 2011-09-07 15:45:16 PDT
Done. I opted for the former. 

(In reply to comment #7)
> > It does, I'm not sure how to do that? Do I just include a different layout test result as the default and another under the chromium directory?
> 
> In this case, because this test is testing a Chromium WebKit API, I would put the test in the LayoutTest/platform/chromium/fast/repaint/fixed-layout-360x240.html
> 
> That will cause the test to only be run on Chromium.  Another approach is to add the test to all the other port's Skipped files, which would be appropriate if this was a test for a feature we'd expect other ports to implement eventually.
Comment 10 WebKit Review Bot 2011-09-08 12:19:35 PDT
Comment on attachment 106662 [details]
Patch

Clearing flags on attachment: 106662

Committed r94779: <http://trac.webkit.org/changeset/94779>
Comment 11 WebKit Review Bot 2011-09-08 12:19:40 PDT
All reviewed patches have been landed.  Closing bug.