WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
98527
REGRESSION (
r129478
-
r129480
): http/tests/loading/text-content-type-with-binary-extension.html failing on Apple MountainLion Debug WK2 (Tests)
https://bugs.webkit.org/show_bug.cgi?id=98527
Summary
REGRESSION (r129478-r129480): http/tests/loading/text-content-type-with-binar...
Jer Noble
Reported
2012-10-05 08:59:46 PDT
http/tests/loading/text-content-type-with-binary-extension.html started failing on Apple MountainLion Debug WK2 (Tests) between
r129479
and
r129480
(inclusive).
http://trac.webkit.org/log/trunk?rev=129480&stop_rev=129479&limit=3
Suspect revision:
http://trac.webkit.org/changeset/129479
http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r129478%20(1444)/results.html
passed
http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r129480%20(1445)/results.html
failed
Attachments
patch
(9.08 KB, patch)
2012-10-11 02:00 PDT
,
Mikhail Pozdnyakov
no flags
Details
Formatted Diff
Diff
patch v2
(7.64 KB, patch)
2012-10-11 05:34 PDT
,
Mikhail Pozdnyakov
kenneth
: review+
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
to be landed
(7.69 KB, patch)
2012-10-11 06:28 PDT
,
Mikhail Pozdnyakov
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Kenneth Rohde Christiansen
Comment 1
2012-10-05 09:02:51 PDT
Misha can you please have a look at this?
Jer Noble
Comment 2
2012-10-05 11:10:28 PDT
It looks like the patch in question is insufficient. After the web process returns "pass through" in response to InjectedBundlePage::decidePolicyForResponse(), it then queries the UIProcess through InjectedBundlePage::decidePolicyForResponse(). Because WKTR has not installed a WKPagePolicyClient, it defaults to "use", and the page loads the binary data.
Jer Noble
Comment 3
2012-10-05 11:41:00 PDT
Added this test to TestExpectations as Failing in
r130536
.
Mikhail Pozdnyakov
Comment 4
2012-10-09 04:41:26 PDT
Think the described problem can be fixed with the same patch with
bug#95974
Mikhail Pozdnyakov
Comment 5
2012-10-09 06:35:39 PDT
(In reply to
comment #4
)
> Think the described problem can be fixed with the same patch with
bug#95974
Just uploaded patch for
bug#95974
that contains partial fix for this bug -- it's introducing PagePolicyClient for WTR. Should also add decidePolicyForResponse callback to fix this bug problem.
Mikhail Pozdnyakov
Comment 6
2012-10-11 02:00:50 PDT
Created
attachment 168173
[details]
patch
Mikhail Pozdnyakov
Comment 7
2012-10-11 02:02:09 PDT
***
Bug 98994
has been marked as a duplicate of this bug. ***
Kenneth Rohde Christiansen
Comment 8
2012-10-11 04:05:22 PDT
Comment on
attachment 168173
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=168173&action=review
> LayoutTests/platform/efl-wk2/http/tests/loading/text-content-type-with-binary-extension-expected.txt:10 > +Binary data not loaded as text. Maybe PASS.
Maybe PASS?
> Tools/WebKitTestRunner/TestController.cpp:1081 > + if (WKURLResponseHTTPStatusCode(response) == 204) {
We have defines for these numbers somewhere in webkit!
> Tools/WebKitTestRunner/TestController.cpp:1087 > + // If the URL Response has "Content-Disposition: attachment;" header, then > + // we should download it.
keep on one line
> Tools/WebKitTestRunner/TestController.cpp:1108 > + // We should ignore downloadable top-level content for subframes, with an exception for text/xml and application/xml so we can still support Acid3 test. > + // It makes the browser intentionally behave differently when it comes to text(application)/xml content in subframes vs. mainframe.
the browser? this is the test system, is this behavior even tested?
Mikhail Pozdnyakov
Comment 9
2012-10-11 04:21:26 PDT
Comment on
attachment 168173
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=168173&action=review
>> LayoutTests/platform/efl-wk2/http/tests/loading/text-content-type-with-binary-extension-expected.txt:10 >> +Binary data not loaded as text. Maybe PASS. > > Maybe PASS?
That is test's expected output :)
>> Tools/WebKitTestRunner/TestController.cpp:1081 >> + if (WKURLResponseHTTPStatusCode(response) == 204) { > > We have defines for these numbers somewhere in webkit!
in WebCore.. WebCore/HTTPStatusCodes.h. Are we supposed to use WebCore data types in WTR?
>> Tools/WebKitTestRunner/TestController.cpp:1087 >> + // we should download it. > > keep on one line
sure.
>> Tools/WebKitTestRunner/TestController.cpp:1108 >> + // It makes the browser intentionally behave differently when it comes to text(application)/xml content in subframes vs. mainframe. > > the browser? this is the test system, is this behavior even tested?
Checking which layout tests should cover it.. Thanks for having a look!
Mikhail Pozdnyakov
Comment 10
2012-10-11 05:33:04 PDT
> > + // It makes the browser intentionally behave differently when it comes to text(application)/xml content in subframes vs. mainframe. > > the browser? this is the test system, is this behavior even tested?
Furthermore, the response has been already handled by WKBundlePagePolicyClient so no need in that complicetd logic here simple ignoring should be enough.
Mikhail Pozdnyakov
Comment 11
2012-10-11 05:34:04 PDT
Created
attachment 168200
[details]
patch v2 Simplified TestController::decidePolicyForResponse().
WebKit Review Bot
Comment 12
2012-10-11 06:04:36 PDT
Comment on
attachment 168200
[details]
patch v2 Rejecting
attachment 168200
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: Kit/chromium/third_party/yasm/source/patched-yasm --revision 154708 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 51>At revision 154708. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output:
http://queues.webkit.org/results/14262134
Mikhail Pozdnyakov
Comment 13
2012-10-11 06:28:28 PDT
Created
attachment 168211
[details]
to be landed
WebKit Review Bot
Comment 14
2012-10-11 07:24:37 PDT
Comment on
attachment 168211
[details]
to be landed Clearing flags on attachment: 168211 Committed
r131057
: <
http://trac.webkit.org/changeset/131057
>
WebKit Review Bot
Comment 15
2012-10-11 07:24:42 PDT
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 16
2012-10-12 01:34:43 PDT
(In reply to
comment #14
)
> (From update of
attachment 168211
[details]
) > Clearing flags on attachment: 168211 > > Committed
r131057
: <
http://trac.webkit.org/changeset/131057
>
It caused a regression on Qt-WK2 -
https://bugs.webkit.org/show_bug.cgi?id=99152
Jessie Berlin
Comment 17
2013-02-04 16:32:53 PST
http/tests/loading/text-content-type-with-binary-extension.htm is showing up as an unexpected pass on the Mac WK2 bots. I am going to remove the failure test expectation for mac-wk2.
Jessie Berlin
Comment 18
2013-02-04 16:44:36 PST
Remove the failing expectation for mac-wk2 in
http://trac.webkit.org/changeset/141831
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug