For testing we use the same fonts and the same RenderTheme for WK1 and WK2. I think the default sizes for forms should be the same as well, so we can keep the same expected results for both WK1 and WK2. I identified this problem while rebaselining editing/inserting for the new fonts. Two tests failing in WK2 because of this are: editing/inserting/4960120-1.html editing/inserting/before-after-input-element.html
tables/mozilla_expected_failures/bugs/bug92647-1.html
css3/selectors3/html/css3-modsel-23.html css3/selectors3/html/css3-modsel-24.html css3/selectors3/html/css3-modsel-68.html css3/selectors3/html/css3-modsel-69.html css3/selectors3/xhtml/css3-modsel-23.xml css3/selectors3/xhtml/css3-modsel-24.xml css3/selectors3/xhtml/css3-modsel-68.xml css3/selectors3/xhtml/css3-modsel-69.xml css3/selectors3/xml/css3-modsel-23.xml css3/selectors3/xml/css3-modsel-24.xml css3/selectors3/xml/css3-modsel-68.xml css3/selectors3/xml/css3-modsel-69.xml
editing/selection/3690703-2.html editing/selection/3690703.html editing/selection/3690719.html (and more to come from previous rebaselines)
editing/selection/4895428-3.html editing/selection/4975120.html editing/selection/drag-select-1.html
fast/forms/basic-inputs.html fast/forms/basic-textareas-quirks.html fast/forms/control-restrict-line-height.html fast/forms/encoding-test.html fast/forms/fieldset-align.html fast/forms/input-appearance-bkcolor.html fast/forms/input-appearance-default-bkcolor.html fast/forms/input-appearance-disabled.html fast/forms/input-appearance-focus.html fast/forms/input-appearance-preventDefault.html fast/forms/input-appearance-readonly.html fast/forms/input-appearance-selection.html fast/forms/input-appearance-visibility.html fast/forms/input-baseline.html fast/forms/input-double-click-selection-gap-bug.html fast/forms/input-placeholder-visibility-1.html fast/forms/input-placeholder-visibility-3.html fast/forms/input-readonly-dimmed.html fast/forms/input-readonly-empty.html fast/forms/input-spaces.html fast/forms/input-text-click-inside.html fast/forms/input-text-double-click.html fast/forms/input-text-option-delete.html fast/forms/input-text-self-emptying-click.html fast/forms/input-text-word-wrap.html fast/forms/input-width.html fast/forms/placeholder-position.html fast/forms/search-cancel-button-style-sharing.html fast/forms/search-display-none-cancel-button.html fast/forms/search-rtl.html fast/forms/search-vertical-alignment.html fast/forms/text-style-color.html fast/forms/textarea-placeholder-pseudo-style.html fast/forms/textarea-placeholder-visibility-1.html fast/forms/textarea-placeholder-visibility-2.html fast/forms/textfield-focus-ring.html fast/forms/textfield-outline.html fast/forms/textfield-overflow.html
fast/text/international/unicode-bidi-plaintext-in-textarea.html fast/text/textIteratorNilRenderer.html
fast/table/colspanMinWidth.html fast/table/spanOverlapRepaint.html fast/table/text-field-baseline.html
tables/mozilla/bugs/bug1188.html tables/mozilla/bugs/bug18359.html tables/mozilla/bugs/bug24200.html tables/mozilla/bugs/bug2479-3.html tables/mozilla/bugs/bug28928.html tables/mozilla/bugs/bug30559.html tables/mozilla/bugs/bug4382.html tables/mozilla/bugs/bug4527.html tables/mozilla/bugs/bug46368-1.html tables/mozilla/bugs/bug46368-2.html tables/mozilla/bugs/bug51037.html tables/mozilla/bugs/bug55545.html tables/mozilla/bugs/bug59354.html tables/mozilla/bugs/bug96334.html tables/mozilla/dom/tableDom.html tables/mozilla/other/move_row.html
There are only 83 tests related to this bug. I think we can simple check in qt-5.0-wk2 specific results to have better test coverage until find a proper fix this bug. ( or feauture? :) )
(In reply to comment #9) > There are only 83 tests related to this bug. I think we can simple check in qt-5.0-wk2 specific results to have better test coverage until find a proper fix this bug. ( or feauture? :) ) I don't think it's a good idea, given that the Luciano is already working on a fix for this and there's no essential reason to WK2 behave differently than WK1 in those cases. In general I think having WK2 specific results should be seen as the last option, only have them if we have a very good reason.
I didn't notice if Luciano is working on it. Of course having same results for WK1 and WK2 is the best solution. :) But if we don't find the proper solution for a long time, we should commit WK2 specific expected results too not to lose test coverage for these tests. The results for WK2 aren't wrong, only a little bit different.
I'm investigating and one of my findings is that DRT and WTR use different fonts (Liberation Serif / Times). Now I'm trying to implement the functions inside webkit/Source/WebKit2/UIProcess/qt/WebPreferencesQt.cpp to make it use the same font as DRT. The current implementation uses default values from webkit/Source/WebKit2/Shared/WebPreferencesStore.h. I hope to close this bug till this weekend.
Created attachment 156686 [details] Use correct fonts from testfonts directory
Comment on attachment 156686 [details] Use correct fonts from testfonts directory View in context: https://bugs.webkit.org/attachment.cgi?id=156686&action=review Here are my informal complaining comments :) > Source/WebKit2/UIProcess/qt/WebPreferencesQt.cpp:56 > +#define INITIALIZE_PREFERENCE_FROM_NSUSERDEFAULTS(KeyUpper, KeyLower, TypeName, Type, DefaultValue) \ > + set##TypeName##ValueIfInUserDefaults(WebPreferencesKey::KeyLower##Key(), m_store, qFontHint); This is a crasy name for this macro :) > Tools/WebKitTestRunner/qt/main.cpp:124 > + WebKit::initializeTestFonts(); > + QCoreApplication::setAttribute(Qt::AA_Use96Dpi, true); > + Does that mean that we draw input-tet and text-area on the UI side? Otherwise it's not clear to me why this is necessary.
(In reply to comment #14) > (From update of attachment 156686 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=156686&action=review > > Here are my informal complaining comments :) > > > Source/WebKit2/UIProcess/qt/WebPreferencesQt.cpp:56 > > +#define INITIALIZE_PREFERENCE_FROM_NSUSERDEFAULTS(KeyUpper, KeyLower, TypeName, Type, DefaultValue) \ > > + set##TypeName##ValueIfInUserDefaults(WebPreferencesKey::KeyLower##Key(), m_store, qFontHint); > > This is a crasy name for this macro :) I took it from Mac implementation :) I'll rename it to something better. > > > Tools/WebKitTestRunner/qt/main.cpp:124 > > + WebKit::initializeTestFonts(); > > + QCoreApplication::setAttribute(Qt::AA_Use96Dpi, true); > > + > > Does that mean that we draw input-tet and text-area on the UI side? Otherwise it's not clear to me why this is necessary. If we don't initialize fontconfig stuff before reaching that macro QFont.defaultFamily returns different (Dejavu font type) results. The way I found to fix this behaviour was to initialize here - as DumpRenderTree does.
Created attachment 156717 [details] Macro has been renamed
Created attachment 156746 [details] Patch for landing
Comment on attachment 156746 [details] Patch for landing Clearing flags on attachment: 156746 Committed r124808: <http://trac.webkit.org/changeset/124808>
All reviewed patches have been landed. Closing bug.
(In reply to comment #18) > (From update of attachment 156746 [details]) > Clearing flags on attachment: 156746 > > Committed r124808: <http://trac.webkit.org/changeset/124808> You forgot to unskip tests and remove unnecessary qt-5.0-wk2 expected files. I did it - https://trac.webkit.org/changeset/124873
(In reply to comment #20) > (In reply to comment #18) > > (From update of attachment 156746 [details] [details]) > > Clearing flags on attachment: 156746 > > > > Committed r124808: <http://trac.webkit.org/changeset/124808> > > You forgot to unskip tests and remove unnecessary qt-5.0-wk2 expected files. > I did it - https://trac.webkit.org/changeset/124873 oops! Thanks a lot!