Bug 82700

Summary: [Qt][WK2] REGRESSION, fix a failing API test in qmltests
Product: WebKit Reporter: Csaba Osztrogon√°c <ossy@webkit.org>
Component: Tools / TestsAssignee: Marcelo Lira <marcelo.lira@openbossa.org>
Status: RESOLVED FIXED    
Severity: Critical CC: alexis@webkit.org, cmarcelo@webkit.org, hausmann@webkit.org, ossy@webkit.org, rafael.lobo@webkit.org, webkit.review.bot@gmail.com, zoltan@webkit.org
Priority: P1 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 70236    
Attachments:
Description Flags
Patch
none
Patch none

Description From 2012-03-30 00:14:51 PST
All API tests pass with the latest pinned Qt5 hash (7d0327830bb4768753cb0c14a23d98fed08be7d2),
but the following test fails with a newer Qt5: 15f3399576bd433898f8e89bbcb3c1196e0da5b9

FAIL!  : qmltests::WebViewPreferences::test_standardFontFamilyChanged() Compared values are not the same
   Actual   (): 'Bitstream Vera Serif'
   Expected (): Bitstream Vera Serif
   Loc: [/home/oszi/WebKit/Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_preferences.qml(204)]
------- Comment #1 From 2012-05-21 12:17:37 PST -------
Created an attachment (id=143075) [details]
Patch
------- Comment #2 From 2012-05-21 12:29:52 PST -------
(From update of attachment 143075 [details])
Question : why the behavior changed in Qt?
------- Comment #3 From 2012-05-21 12:33:49 PST -------
(From update of attachment 143075 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=143075&action=review

Thanks for finally fixing this one, Marcelo :-)  I have some comments following.

> Source/WebKit2/ChangeLog:10
> +        Font family names returned by WebKit experimental always return values

I would rephrase this, at least to make clear is our: __WebView__ experimental preferences API. And we return the same values as existing WKPreferences C API for WK2.

> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_preferences.qml:199
> +                return text.replace(/^('*)([^']*)('*)$/, '$2')

Can we make it simpler? Like /^'?(.*)'?$/, '$1'... or even no Regexp at all.

This still doesn't support fonts that had ' originally in the name, but since our set of fonts is controlled for testing, that should be OK.

> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_preferences.qml:210
> +                compare(unquote(webView.title), unquote(defaultStandardFontFamily))

Just need to unquote the webView.title here since we expect defaultStandarFontFamily to be unquoted.
------- Comment #4 From 2012-05-21 12:37:29 PST -------
(From update of attachment 143075 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=143075&action=review

>> Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_preferences.qml:199
>> +                return text.replace(/^('*)([^']*)('*)$/, '$2')
> 
> Can we make it simpler? Like /^'?(.*)'?$/, '$1'... or even no Regexp at all.
> 
> This still doesn't support fonts that had ' originally in the name, but since our set of fonts is controlled for testing, that should be OK.

Bah. My suggestion is wrong because (.*) will eat everything. But I still think we can simplify this.
------- Comment #5 From 2012-05-21 12:54:54 PST -------
Created an attachment (id=143079) [details]
Patch
------- Comment #6 From 2012-05-21 12:56:26 PST -------
Looks good to me.
------- Comment #7 From 2012-05-21 12:57:49 PST -------
(From update of attachment 143079 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=143079&action=review

> Source/WebKit2/ChangeLog:9
> +        quotes when white space is present in its name, and no quotes otherwise.

Is it on the spec?
------- Comment #8 From 2012-05-21 13:01:53 PST -------
(In reply to comment #7)
> (From update of attachment 143079 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143079&action=review
> 
> > Source/WebKit2/ChangeLog:9
> > +        quotes when white space is present in its name, and no quotes otherwise.
> 
> Is it on the spec?

Yes
------- Comment #9 From 2012-05-21 13:03:58 PST -------
(In reply to comment #7)
> (From update of attachment 143079 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=143079&action=review
> 
> > Source/WebKit2/ChangeLog:9
> > +        quotes when white space is present in its name, and no quotes otherwise.
> 
> Is it on the spec?

"To avoid mistakes in escaping, it is recommended to quote font family names that contain white space, digits, or punctuation characters other than hyphens (...)"

http://www.w3.org/TR/CSS21/fonts.html#font-family-prop
------- Comment #10 From 2012-05-21 13:47:35 PST -------
(From update of attachment 143079 [details])
Clearing flags on attachment: 143079

Committed r117812: <http://trac.webkit.org/changeset/117812>
------- Comment #11 From 2012-05-21 13:47:40 PST -------
All reviewed patches have been landed.  Closing bug.