EFL, Qt and GTK ports use a default minimum logical font size of 0 for WebKit1, while Mac and Windows use 9. On WebKit2, the default value is 9 for all platforms. This results in tests having different output with WK1 and WK2 due to text being bigger with WK2 than with WK1 (for EFL, GTK and Qt ports). At least the following test cases are affected: fast/ruby/nested-ruby.html fast/ruby/ruby-beforeafter.html fast/ruby/ruby-empty-rt.html fast/ruby/ruby-length.html fast/ruby/ruby-run-break.html fast/ruby/ruby-runs-spans.html fast/ruby/ruby-runs.html fast/ruby/ruby-simple-rp.html fast/ruby/ruby-simple.html fast/ruby/ruby-trailing.html fast/ruby/rubyDOM-insert-rt.html fast/ruby/rubyDOM-insert-text1.html fast/ruby/rubyDOM-insert-text2.html fast/ruby/rubyDOM-insert-text3.html fast/ruby/rubyDOM-remove-rt1.html fast/ruby/rubyDOM-remove-rt2.html fast/ruby/rubyDOM-remove-text1.html fast/ruby/rubyDOM-remove-text2.html http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm We need to fix this to have consistent values for all ports between WK1 and WK2.
Created attachment 162791 [details] Patch
Adding Ossy in CC since I haven't unskipped the tests for Qt port. I did not dare unskip those tests for Qt ports because they are skipped in the global Skipped list, instead of the wk2 one. Moreover, I don't have a local Qt port build to verify if the tests are now passing.
Created attachment 162793 [details] Patch Rebase on master.
Comment on attachment 162791 [details] Patch Wouldn't it make sense to set this explicitly in the test runner instead of changing the default? It would just be a matter of updating both DumpRenderTree and WebKitTestRunner.
(In reply to comment #4) > (From update of attachment 162791 [details]) > Wouldn't it make sense to set this explicitly in the test runner instead of changing the default? It would just be a matter of updating both DumpRenderTree and WebKitTestRunner. It is not just about layout tests. I don't see why we would use a different default value for the same setting in WebKit1 and WebKit2. Sure, we can set them explicitly in DRT / WKTR but it does not change the fact that the default value should be consistent between WK1 and WK2 for each port IMHO. There are already other settings in the file I edited that are customized for each port using #if statements, for e.g.: #if PLATFORM(GTK) #define DEFAULT_WEBKIT_TABSTOLINKS_ENABLED true #else #define DEFAULT_WEBKIT_TABSTOLINKS_ENABLED false #endif
(In reply to comment #5) > Sure, we can set them explicitly in DRT / WKTR but it does not change the fact that the default value should be consistent between WK1 and WK2 for each port IMHO. Since the WebKit2 C API is designed to be cross-platform, wouldn't it make more sense to value the consistency among the WebKit2 C APIs higher than the consistency of particular port's WebKit1 and WebKit2 ports? Note that what we are talking about it only for the C API. Ports may choose to adjust the settings for their user-exposed APIs (for GTK+ this is the GObject one). Consider that reducing the differences between the C APIs reduces the amount of platform-dependent code in WebKitTestRunner. In the past, what we've done is to bring DumpRenderTree in line with the output of WebKitTestRunner, which brings all results closer to each other. This change seems to only solidify those differences. > There are already other settings in the file I edited that are customized for each port using #if statements, for e.g.: > > #if PLATFORM(GTK) > #define DEFAULT_WEBKIT_TABSTOLINKS_ENABLED true > #else > #define DEFAULT_WEBKIT_TABSTOLINKS_ENABLED false > #endif I think in this case especially, we should probably move this to the GObject API layer.
Created attachment 162839 [details] Patch Take Martin Robinson's feedback into consideration. Dropped support for other ports since this approach requires rebaselining.
Comment on attachment 162839 [details] Patch Clearing flags on attachment: 162839 Committed r127909: <http://trac.webkit.org/changeset/127909>
All reviewed patches have been landed. Closing bug.