Bug 60236

Summary:

full-screen-stacking-context-expected.txt should be under platform/mac

Product:

WebKit

Reporter:

Hajime Morrita <morrita>

Component:

Tools / Tests

Assignee:

Hajime Morrita <morrita>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, eric, jer.noble, koivisto, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

60237

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Hajime Morrita

Reported 2011-05-04 18:33:22 PDT

A test result commited at r85811 contains the layout dump, which is platform dependent. We should move it to platform directly.

Attachments
Patch (728 bytes, patch) 2011-05-04 18:38 PDT, Hajime Morrita	no flags	Details Formatted Diff Diff
Patch (5.38 KB, patch) 2011-05-05 11:32 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Patch (11.10 KB, patch) 2011-05-05 13:18 PDT, Jer Noble	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Hajime Morrita

Comment 1 2011-05-04 18:38:04 PDT

Created attachment 92356 [details] Patch

Kent Tamura

Comment 2 2011-05-04 20:42:03 PDT

Comment on attachment 92356 [details] Patch The patch has only ChangeLog.

Hajime Morrita

Comment 3 2011-05-04 21:04:21 PDT

Hi Kent-san, thank you for taking a look! (In reply to comment #2) > (From update of attachment 92356 [details]) > The patch has only ChangeLog. Hmm. It seems webkit-patch post doesn't support simple file move... I didn't know that.

Jer Noble

Comment 4 2011-05-04 23:11:57 PDT

(In reply to comment #0) > A test result commited at r85811 contains the layout dump, which is platform dependent. > We should move it to platform directly. Why is the layout dump platform dependant? The pixel test, perhaps, but the layout itself should be platform agnostic.

Jer Noble

Comment 5 2011-05-04 23:15:26 PDT

Note that other tests in the LayoutTests/fullscreen/ directory have pixel test results in LayoutTests/platform/mac/fullscreen/ and layout dumps within LayoutTests/fullscreen/, notably the full-screen-zIndex.html test.

Tony Chang

Comment 6 2011-05-05 10:41:46 PDT

(In reply to comment #4) > Why is the layout dump platform dependant? The pixel test, perhaps, but the layout itself should be platform agnostic. Any test that has text output will likely differ by platforms since the font metrics are different. For example, full-screen-zIndex.html has separate -expected.txt results for chromium-win, chromium-linux and gtk. The test is skipped on win and qt.

Jer Noble

Comment 7 2011-05-05 11:04:24 PDT

(In reply to comment #6) > (In reply to comment #4) > > Why is the layout dump platform dependant? The pixel test, perhaps, but the layout itself should be platform agnostic. > > Any test that has text output will likely differ by platforms since the font metrics are different. Ah, I see, so the bounds of each text line dumped will be different. Should these tests be re-written to remove the text portions?

Tony Chang

Comment 8 2011-05-05 11:19:35 PDT

(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #4) > > > Why is the layout dump platform dependant? The pixel test, perhaps, but the layout itself should be platform agnostic. > > > > Any test that has text output will likely differ by platforms since the font metrics are different. > > Ah, I see, so the bounds of each text line dumped will be different. Should these tests be re-written to remove the text portions? Another option is to use layoutTestController.dumpAsText(true). This tells DRT to use the dumpAsText output + include a pixel result.

Alexey Proskuryakov

Comment 9 2011-05-05 11:21:49 PDT

> Should these tests be re-written to remove the text portions? If that can be done without making the tests harder to run manually, and the tests can't be text-only, then that's definitely the best solution. My dream is to have a font that's rendered by all platforms identically :)

Jer Noble

Comment 10 2011-05-05 11:26:12 PDT

(In reply to comment #8) > Another option is to use layoutTestController.dumpAsText(true). This tells DRT to use the dumpAsText output + include a pixel result. This seems like the optimal scenario. I'll look into doing this.

Jer Noble

Comment 11 2011-05-05 11:32:53 PDT

Created attachment 92435 [details] Patch

Tony Chang

Comment 12 2011-05-05 11:49:47 PDT

Comment on attachment 92435 [details] Patch Can you delete the other platform/*/fullscreen/full-screen-zIndex-expected.txt files in this patch? Otherwise, this looks good.

Jer Noble

Comment 13 2011-05-05 13:18:44 PDT

Created attachment 92458 [details] Patch

Tony Chang

Comment 14 2011-05-05 14:20:33 PDT

Comment on attachment 92458 [details] Patch Thanks!

Jer Noble

Comment 15 2011-05-05 14:30:44 PDT

Comment on attachment 92458 [details] Patch Adding cq+.

WebKit Commit Bot

Comment 16 2011-05-05 16:06:41 PDT

The commit-queue encountered the following flaky tests while processing attachment 92458 [details]: java/lc3/JavaObject/JavaObjectToLong-005.html bug 60309 (author: ap@webkit.org) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 17 2011-05-05 16:08:08 PDT

Comment on attachment 92458 [details] Patch Clearing flags on attachment: 92458 Committed r85897: <http://trac.webkit.org/changeset/85897>

WebKit Commit Bot

Comment 18 2011-05-05 16:08:13 PDT

All reviewed patches have been landed. Closing bug.

WebKit Review Bot

Comment 19 2011-05-05 17:23:52 PDT

http://trac.webkit.org/changeset/85897 might have broken GTK Linux 32-bit Release The following tests are not passing: fullscreen/full-screen-stacking-context.html

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