Bug 98729 – REGRESSION (r130699): 5 various fast/ tests started failing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Zan Dobersek]

The following tests started failing after r130699:
http://trac.webkit.org/changeset/130699

fast/css/import-style-update.html
fast/html/link-rel-stylesheet.html
fast/loader/data-url-encoding-html.html
fast/loader/data-url-encoding-svg.html
fast/spatial-navigation/snav-iframe-nested.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fcss%2Fimport-style-update.html%20fast%2Fhtml%2Flink-rel-stylesheet.html%20fast%2Floader%2Fdata-url-encoding-html.html%20fast%2Floader%2Fdata-url-encoding-svg.html%20fast%2Fspatial-navigation%2Fsnav-iframe-nested.html

[Zan Dobersek]

Narrowing down the offending revision was done through the help of EFL buildbots since the GTK ones were failing at the compilation step. The flakiness dashboard has that data.

[Zan Dobersek]

Also adding svg/custom/object-data-href.html under this bug, though I'm not 100% the failure stems from this commit. The EFL port doesn't seem to fail this test.

[Zan Dobersek]

... and 4 tests timing out as well. These are failing on EFL as well.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=fast%2Fevents%2Fattribute-listener

fast/events/attribute-listener-cloned-from-frameless-doc-context.html
fast/events/attribute-listener-cloned-from-frameless-doc-context-2.html
fast/events/attribute-listener-extracted-from-frameless-doc-context.html
fast/events/attribute-listener-extracted-from-frameless-doc-context-2.html

[Raphael Kubo da Costa (:rakuco)]

I have skipped these tests for the EFL port in <http://trac.webkit.org/changeset/130745>.

Perhaps we should roll the commit out later today if there's no fix in sight?

[Zan Dobersek]

(In reply to comment #4)
> I have skipped these tests for the EFL port in <http://trac.webkit.org/changeset/130745>.
> 
> Perhaps we should roll the commit out later today if there's no fix in sight?

I'd say we should wait for a response from Martin and Kov. They'll make it clear whether they can get to fixing the regressions soon or whether to just rollout this patch (and all the other fixes following it) and fixing it outside of trunk.

[Martin Robinson]

Do you mind skipping the tests for now and I'll try to fix them?

[Zan Dobersek]

(In reply to comment #6)
> Do you mind skipping the tests for now and I'll try to fix them?

Already done shortly after reporting the bug. Happy fixing.

[Martin Robinson]

Created attachment 167816 [details]
Patch

[Dan Winship]

Comment on attachment 167816 [details]
Patch

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

> Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:129
> +    // Data URLs don't have fragment identifiers.

That's not true. The data: URLs in those tests are syntactically incorrect. "#" isn't a valid character in the <data> part of a data: URL; they need "%23". (And presumably KURL is broken if the other ports *aren't* failing those tests...)

https://bugzilla.mozilla.org/show_bug.cgi?id=308590 suggests several SVG features can't work on SVGs loaded from data: URLs unless you deal with fragments on them correctly.

[Martin Robinson]

(In reply to comment #9)
> (From update of attachment 167816 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=167816&action=review
> 
> > Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp:129
> > +    // Data URLs don't have fragment identifiers.
> 
> That's not true. The data: URLs in those tests are syntactically incorrect. "#" isn't a valid character in the <data> part of a data: URL; they need "%23". (And presumably KURL is broken if the other ports *aren't* failing those tests...)

Other ports do not try to remove the fragment identifier from Data URLs, as far as I know.  This is a work-around for soup behavior. See: http://trac.webkit.org/changeset/52305

I can change the comment to:

// KURL does not yet properly handle fragment identifiers in Data URLs, so don't even try.

> https://bugzilla.mozilla.org/show_bug.cgi?id=308590 suggests several SVG features can't work on SVGs loaded from data: URLs unless you deal with fragments on them correctly.

[Martin Robinson]

(In reply to comment #10)

> Other ports do not try to remove the fragment identifier from Data URLs, as far as I know.  This is a work-around for soup behavior. See: http://trac.webkit.org/changeset/52305

This would be more accurately stated as "Other ports do not try to remove the fragment identifier from *any* URLs."

[Dan Winship]

ok, bug 68089 says this parsing behavior is intentional. IRC discussion led to the suggestion that ResourceRequestSoup should translate "#" to "%23" in data: URLs, so that it continues to match this behavior regardless of what libsoup does.

[Martin Robinson]

Created attachment 167857 [details]
Patch

[Dan Winship]

yeah, that looks right to me

[Xan Lopez]

Comment on attachment 167857 [details]
Patch

OK!

[Martin Robinson]

Thanks for the review!

[WebKit Review Bot]

Comment on attachment 167857 [details]
Patch

Rejecting attachment 167857 [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:
1 with fuzz 3.
patching file Source/WebCore/platform/network/soup/ResourceRequestSoup.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/gtk/TestExpectations
Hunk #1 FAILED at 1387.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/gtk/TestExpectations.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Xan Lopez']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

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

[Martin Robinson]

Committed r131942: <http://trac.webkit.org/changeset/131942>