Bug 47609

Summary: [Qt] Fix http/tests/misc/redirect-to-external-url.html
Product: WebKit Reporter: Robert Hogan <robert>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: commit-queue, jwieczorek, tonikitoo
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 47687    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Robert Hogan 2010-10-13 11:48:38 PDT
Support the test in FrameLoaderClientQt
Comment 1 Robert Hogan 2010-10-13 11:51:09 PDT
Created attachment 70642 [details]
Patch
Comment 2 Jakub Wieczorek 2010-10-14 12:32:58 PDT
> [Qt] Fix http/tests/misc/redirect-to-external-url.html

Nah, you're not fixing it, really.
I recommend looking at what other ports are doing but special-casing a specific situation from a test case inside WebKit is not a solution at all. Not to mention that theoretically each scheme can be supported by the embedder via an extended QNAM.

If this patch was OK, we could go ahead and reduce the failing test list to 0 with similar changes, but in real-world situations QtWebKit would still not work as expected and you'd not be aware of that due to the tests happily passing.
Comment 3 WebKit Commit Bot 2010-10-14 12:38:34 PDT
Comment on attachment 70642 [details]
Patch

Clearing flags on attachment: 70642

Committed r69795: <http://trac.webkit.org/changeset/69795>
Comment 4 WebKit Commit Bot 2010-10-14 12:38:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Antonio Gomes 2010-10-14 12:42:47 PDT
(In reply to comment #2)
> > [Qt] Fix http/tests/misc/redirect-to-external-url.html
> 
> Nah, you're not fixing it, really.
> I recommend looking at what other ports are doing but special-casing a specific situation from a test case inside WebKit is not a solution at all. Not to mention that theoretically each scheme can be supported by the embedder via an extended QNAM.
> 
> If this patch was OK, we could go ahead and reduce the failing test list to 0 with similar changes, but in real-world situations QtWebKit would still not work as expected and you'd not be aware of that due to the tests happily passing.

Oh. should we roll it out?

it is one command on IRC :)
Comment 6 Robert Hogan 2010-10-14 12:58:37 PDT
(In reply to comment #2)
> > [Qt] Fix http/tests/misc/redirect-to-external-url.html
> 
> Nah, you're not fixing it, really.
> I recommend looking at what other ports are doing but special-casing a specific situation from a test case inside WebKit is not a solution at all. Not to mention that theoretically each scheme can be supported by the embedder via an extended QNAM.

Chromium special-cases it. My understanding is that the test is for WebCore functionality, PolicyChecker::continueAfterNavigationPolicy(PolicyAction policy and the canHandleRequest() is expected to fail.

I agree that it would be better to remove the DRT special-casing we have in FrameLoaderClientQt, along with all the other DRT specific messages in there. But that's a different problem.
Comment 7 Jakub Wieczorek 2010-10-14 13:18:55 PDT
(In reply to comment #6)
> (In reply to comment #2)
> > > [Qt] Fix http/tests/misc/redirect-to-external-url.html
> > 
> > Nah, you're not fixing it, really.
> > I recommend looking at what other ports are doing but special-casing a specific situation from a test case inside WebKit is not a solution at all. Not to mention that theoretically each scheme can be supported by the embedder via an extended QNAM.
> 
> Chromium special-cases it. My understanding is that the test is for WebCore functionality, PolicyChecker::continueAfterNavigationPolicy(PolicyAction policy and the canHandleRequest() is expected to fail.

OK. I don't think you should follow Chromium in this case though. Other ports don't seem to do this and yet, they pass this test, so apparently they can get the necessary information from the network stack. QtWebKit, due to its abstract network interface, cannot.

Moreover, if a specific behavior of canHandleRequest() is an assumption in this test case and yet that behavior never occurs in QtWebKit, what's the point of testing this anyway?

I think all this test deserves is a good comment in the skipped list.

> I agree that it would be better to remove the DRT special-casing we have in FrameLoaderClientQt, along with all the other DRT specific messages in there. But that's a different problem.

Yes. I'd suggest we don't make the situation even worse though.
Comment 8 Antonio Gomes 2010-10-14 21:11:55 PDT
Rolled out by bug 47687.
Comment 9 Jocelyn Turcotte 2014-02-03 03:10:25 PST
=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.