Bug 56924

Summary:

[Qt] Fix Api tests for QWebPage on symbian

Product:

WebKit

Reporter:

Yi Shen <max.hong.shen>

Component:

WebKit Qt

Assignee:

Yi Shen <max.hong.shen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ademar, commit-queue, laszlo.gombos, menard

Priority:

Version:

528+ (Nightly build)

Hardware:

S60 Hardware

OS:

Other

Bug Depends on:

Bug Blocks:

50925

Attachments:

Description	Flags
first try	menard: review-, menard: commit-queue-
updated with Alexis's suggestion	laszlo.gombos: review-
ignores the style	none
fix the regular expression	none

Yi Shen

Reported 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.

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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Yi Shen

Comment 1 2011-03-23 08:46:12 PDT

Created attachment 86620 [details] first try

Alexis Menard (darktears)

Comment 2 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.

Yi Shen

Comment 3 2011-03-30 12:55:46 PDT

Created attachment 87599 [details] updated with Alexis's suggestion

Alexis Menard (darktears)

Comment 4 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

Laszlo Gombos

Comment 5 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 ?

Yi Shen

Comment 6 2011-04-15 14:32:25 PDT

Created attachment 89855 [details] ignores the style

Yi Shen

Comment 7 2011-04-18 11:54:39 PDT

Created attachment 90063 [details] fix the regular expression

Laszlo Gombos

Comment 8 2011-04-18 12:14:11 PDT

Comment on attachment 90063 [details] fix the regular expression r=me. Thanks.

WebKit Commit Bot

Comment 9 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>

WebKit Commit Bot

Comment 10 2011-04-18 12:49:56 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.