Patch will be added.
Created attachment 124501 [details] Patch
Comment on attachment 124501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124501&action=review I wonder how many viewport test cases are passed by this patch. > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:403 > + printf("viewport size %dx%d scale %f with limits [%f, %f] and userScalable %f\n", attributes.layoutSize.width(), attributes.layoutSize.height(), attributes.initialScale, attributes.minimumScale, attributes.maximumScale, attributes.userScalable); printf can influence on layout test result. Remove it or change other thing.
(In reply to comment #2) > (From update of attachment 124501 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=124501&action=review > > I wonder how many viewport test cases are passed by this patch. > 143 test cases are passed with this. > > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:403 > > + printf("viewport size %dx%d scale %f with limits [%f, %f] and userScalable %f\n", attributes.layoutSize.width(), attributes.layoutSize.height(), attributes.initialScale, attributes.minimumScale, attributes.maximumScale, attributes.userScalable); > > printf can influence on layout test result. Remove it or change other thing. It is what I want, because The expected result of viewport tests check above. Gtk use fprintf(stdout, ... and Qt use fputs for it. I found that DRT/Efl are using printf for the test result, so I choosed printf.
Comment on attachment 124501 [details] Patch Personally, I would like to avoid to use printf directly. Because, IMHO, printf can make us confuse if this is real debug message or not. Though tiled backing store files are using printf in efl port now, I think the printf should be replaced by other's.
(In reply to comment #4) > (From update of attachment 124501 [details]) > Personally, I would like to avoid to use printf directly. Because, IMHO, printf can make us confuse if this is real debug message or not. Though tiled backing store files are using printf in efl port now, I think the printf should be replaced by other's. I used printf because DRT/Efl are using printf in other area. But, printf, fprintf, fputs are not important for me. If you and kubo and other guys like fprintf more, I can revise the patch. Kubo, how do you think about it ?
Just a few fixes may be required. > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:389 > +void DumpRenderTreeSupportEfl::dumpConfigurationForViewport(Evas_Object* ewkView, int deviceDPI, const WebCore::IntSize& deviceSize, const WebCore::IntSize& availableSize) Why don't you use const type for deviceDPI ? > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:397 > + /*default layout width for non-mobile pages */ 980, /*default <= check this ? > LayoutTests/ChangeLog:8 > + Removed fast/viewport from Skipped and add some tests which is not passed. Removed -> Remove :) is not passed -> aren't passed
Created attachment 124662 [details] Patch
Created attachment 124663 [details] Patch
(In reply to comment #6) > Just a few fixes may be required. > > > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:389 > > +void DumpRenderTreeSupportEfl::dumpConfigurationForViewport(Evas_Object* ewkView, int deviceDPI, const WebCore::IntSize& deviceSize, const WebCore::IntSize& availableSize) > > Why don't you use const type for deviceDPI ? Does it have any benefits ? IMO, it's not problem. > > > Source/WebKit/efl/WebCoreSupport/DumpRenderTreeSupportEfl.cpp:397 > > + /*default layout width for non-mobile pages */ 980, > > /*default <= check this ? fixed. > > > LayoutTests/ChangeLog:8 > > + Removed fast/viewport from Skipped and add some tests which is not passed. > > Removed -> Remove :) > is not passed -> aren't passed fixed. Thanks for my typo.
Comment on attachment 124663 [details] Patch LGTM. :-)
Comment on attachment 124663 [details] Patch Clearing flags on attachment: 124663 Committed r106431: <http://trac.webkit.org/changeset/106431>
All reviewed patches have been landed. Closing bug.