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...
Attachments
patch (9.08 KB, patch)
2012-10-11 02:00 PDT, Mikhail Pozdnyakov
no flags
patch v2 (7.64 KB, patch)
2012-10-11 05:34 PDT, Mikhail Pozdnyakov
kenneth: review+
webkit.review.bot: commit-queue-
to be landed (7.69 KB, patch)
2012-10-11 06:28 PDT, Mikhail Pozdnyakov
no flags
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
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.