Bug 56924 - [Qt] Fix Api tests for QWebPage on symbian
Summary: [Qt] Fix Api tests for QWebPage on symbian
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: S60 Hardware Other
: P2 Normal
Assignee: Yi Shen
URL:
Keywords:
Depends on:
Blocks: 50925
  Show dependency treegraph
 
Reported: 2011-03-23 07:55 PDT by Yi Shen
Modified: 2011-05-17 06:16 PDT (History)
4 users (show)

See Also:


Attachments
first try (6.07 KB, patch)
2011-03-23 08:46 PDT, Yi Shen
menard: review-
menard: commit-queue-
Details | Formatted Diff | Diff
updated with Alexis's suggestion (6.46 KB, patch)
2011-03-30 12:55 PDT, Yi Shen
laszlo.gombos: review-
Details | Formatted Diff | Diff
ignores the style (4.74 KB, patch)
2011-04-15 14:32 PDT, Yi Shen
no flags Details | Formatted Diff | Diff
fix the regular expression (4.76 KB, patch)
2011-04-18 11:54 PDT, Yi Shen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yi Shen 2011-03-23 07:55:40 PDT
There are several Api test failures for QWebpage on Symbian, which are caused by the font ('NOkia Sans S60') used on Symbian. I will provide a patch for reviewing.
Comment 1 Yi Shen 2011-03-23 08:46:12 PDT
Created attachment 86620 [details]
first try
Comment 2 Alexis Menard (darktears) 2011-03-29 10:34:42 PDT
Comment on attachment 86620 [details]
first try

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

A quick look tells me that the html code used as reference is the same except for the font-family. Could you use instead QString::arg and specify a different arg for either Symbian or other platforms? I'm wondering also if you could get the current font used so that we don't need to add ifdef at all.

> Source/WebKit/qt/ChangeLog:8
> +        Modified test case to use font 'NOkia Sans S60' on Symbian.

Typo :D I believe it's Nokia not NOkia.
Comment 3 Yi Shen 2011-03-30 12:55:46 PDT
Created attachment 87599 [details]
updated with Alexis's suggestion
Comment 4 Alexis Menard (darktears) 2011-03-30 13:02:34 PDT
Comment on attachment 87599 [details]
updated with Alexis's suggestion

Great so much better, need a r+ from a reviewer now :D
Comment 5 Laszlo Gombos 2011-04-15 14:17:41 PDT
Comment on attachment 87599 [details]
updated with Alexis's suggestion

I do not think autotests should be sensitive to platform fonts. I would rather modify the tests so that it simply ignores the style. 

For font-sensitive tests we should use LayoutTests which has a mechanism of platform dependent expected results.

Yi, can you change the test so that it ignores the style that selectedHtml() returns ?
Comment 6 Yi Shen 2011-04-15 14:32:25 PDT
Created attachment 89855 [details]
ignores the style
Comment 7 Yi Shen 2011-04-18 11:54:39 PDT
Created attachment 90063 [details]
fix the regular expression
Comment 8 Laszlo Gombos 2011-04-18 12:14:11 PDT
Comment on attachment 90063 [details]
fix the regular expression

r=me. Thanks.
Comment 9 WebKit Commit Bot 2011-04-18 12:49:49 PDT
Comment on attachment 90063 [details]
fix the regular expression

Clearing flags on attachment: 90063

Committed r84171: <http://trac.webkit.org/changeset/84171>
Comment 10 WebKit Commit Bot 2011-04-18 12:49:56 PDT
All reviewed patches have been landed.  Closing bug.