[Qt] REGRESSION (r72360): http/tests/security/aboutBlank/xss-DENIED-* tests fail

• Component: New Bugs
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

http://build.webkit.org/results/Qt%20Linux%20Release/r72366%20%2823972%29/results.html

diffs are same:
-CONSOLE MESSAGE: line 1: TypeError: 'undefined' is not an object (evaluating 'target.document.body')
+CONSOLE MESSAGE: line 0: TypeError: 'undefined' is not an object

[Csaba Osztrogonác]

I added the failing tests to the Skipped list: http://trac.webkit.org/changeset/72397

[Gavin Barraclough]

Hi Ossy,

This should be an easy fix, but there are a couple of options, and I'm not sure which way we prefer to resolve this.

We've deliberately made a change to remove the source snippet ("(evaluating 'target.document.body')" in this case) from from error messages in release builds, unless the developer extras are enabled (i.e. inspector support is turned on).  So these results match correctly match those a user would see, if the inspector is not showing.  So one valid resolution would be to say, these are the correct results for a release run of this test case, and we should just check in expected results matching these.

However in order to ensure we are still testing the correct generation of these errors when the inspector is up, we're now always enabling the developer extras from DRT (in Qt's case, we do so from DumpRenderTree/qt/DumpRenderTreeQt.cpp).  So an alternative resolution for this bug would be to explore why the developer extras are not be enabled when this test case is run, and change this (I'm guessing it's something to do with this being a http test...?).  I'm not sure this buys us much, since the correct generation of these error messages is already well covered by LayoutTests.

Any thoughts on a preferred solution?

cheers,
G.

[Robert Hogan]

(In reply to comment #2)
> 
> We've deliberately made a change to remove the source snippet ("(evaluating 'target.document.body')" in this case) from from error messages in release builds, unless the developer extras are enabled (i.e. inspector support is turned on).  So these results match correctly match those a user would see, if the inspector is not showing.  

In this case, developerExtras is enabled but the inspector is not showing - hence the failure.

> Any thoughts on a preferred solution?
> 

The new results seem sane - otherwise we'll need to display the web inspector for these tests.

[Robert Hogan]

Created attachment 79096 [details]
Patch

[Robert Hogan]

Created attachment 79097 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 79097 [details]
Patch

Rejecting attachment 79097 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 2

Last 500 characters of output:
te-expected.txt
patching file LayoutTests/platform/qt/http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url-expected.txt
patching file LayoutTests/platform/qt/http/tests/security/aboutBlank/xss-DENIED-set-opener-expected.txt
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christia..." exit_code: 1

Full output: http://queues.webkit.org/results/7605252

[Robert Hogan]

Created attachment 79843 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 79843 [details]
Patch

Clearing flags on attachment: 79843

Committed r76446: <http://trac.webkit.org/changeset/76446>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.