Bug 58779 – REGRESSION (r84010): [Qt] DRT: Unbreak redirection of http:/ URLs.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Andreas Kling]

r84010 broke redirection of [broken] http:/ URLs because KURL cannot extract the host part of such URLs.

[Andreas Kling]

Created attachment 90026 [details]
Proposed patch

[Eric Seidel (no email)]

Comment on attachment 90026 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90026&action=review

> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1029
> +    QUrl url = newRequest.url();

Why use a QUrl instead of a KURL?

[Andreas Kling]

(In reply to comment #2)
> (From update of attachment 90026 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90026&action=review
> 
> > Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1029
> > +    QUrl url = newRequest.url();
> 
> Why use a QUrl instead of a KURL?

See ChangeLog:

"KURL::host() doesn't return the host part of [broken] http:/ URLs, so use QUrl instead to match the behavior of other ports."

I could also fix the test to use http:// instead of http:/ but it seemed more appropriate to have consistent behavior across ports.

[Adam Barth]

Comment on attachment 90026 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90026&action=review

>>> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1029
>>> -    KURL url = newRequest.url();
>>> +    QUrl url = newRequest.url();
>> 
>> Why use a QUrl instead of a KURL?
> 
> See ChangeLog:
> 
> "KURL::host() doesn't return the host part of [broken] http:/ URLs, so use QUrl instead to match the behavior of other ports."
> 
> I could also fix the test to use http:// instead of http:/ but it seemed more appropriate to have consistent behavior across ports.

How do other port solve this problem with KURL?

[Andreas Kling]

(In reply to comment #4)
> (From update of attachment 90026 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90026&action=review
> 
> >>> Source/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1029
> >>> -    KURL url = newRequest.url();
> >>> +    QUrl url = newRequest.url();
> >> 
> >> Why use a QUrl instead of a KURL?
> > 
> > See ChangeLog:
> > 
> > "KURL::host() doesn't return the host part of [broken] http:/ URLs, so use QUrl instead to match the behavior of other ports."
> > 
> > I could also fix the test to use http:// instead of http:/ but it seemed more appropriate to have consistent behavior across ports.
> 
> How do other port solve this problem with KURL?

Apple is using NSURL, Chromium GURL and Gtk is using SoupURI. In other words, everyone is using their platform URL class.

[Adam Barth]

Comment on attachment 90026 [details]
Proposed patch

I think a better solution would be to fix KURL, but this is probably ok for now.

[WebKit Commit Bot]

Comment on attachment 90026 [details]
Proposed patch

Clearing flags on attachment: 90026

Committed r84168: <http://trac.webkit.org/changeset/84168>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/84168 might have broken Qt Linux Release
The following tests are not passing:
fast/ruby/after-block-doesnt-crash.html
fast/ruby/after-table-doesnt-crash.html
fast/ruby/before-block-doesnt-crash.html
fast/ruby/before-table-doesnt-crash.html