Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Location.SetHref for Iframe with no src #20906

Open
gterzian opened this issue Jun 3, 2018 · 3 comments
Open

Fix Location.SetHref for Iframe with no src #20906

gterzian opened this issue Jun 3, 2018 · 3 comments
Labels

Comments

@gterzian
Copy link
Member

@gterzian gterzian commented Jun 3, 2018

test: "/html/browsers/browsing-the-web/navigating-across-documents/005.html"
code:

Err(e) => return Err(Error::Type(format!("Couldn't parse URL: {}", e))),

spec: https://html.spec.whatwg.org/multipage/#attr-iframe-src

Current error: Error at http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/005.html:1:1 Couldn't parse URL: relative URL with a cannot-be-a-base base

I guess one would have to parse the url related to the parent window when there is no src?
Or perhaps it's the test that should be updated, because the spec seems to read that the src should default to "about:blank" if unset.. https://html.spec.whatwg.org/multipage/#otherwise-steps-for-iframe-or-frame-elements

@jdm
Copy link
Member

@jdm jdm commented Jun 3, 2018

That's a fascinating test! For clarity:

  • there is an iframe that is pointed at about:blank
  • there is an <a> element with an onclick that sets the iframe's contentWindow.location to a relative URL
  • this makes Location::SetHref unhappy because about:blank is not regarded as a valid base URL for relative URLs
@jdm
Copy link
Member

@jdm jdm commented Jun 3, 2018

The problem here may actually be the implementation of Location::SetHref. The spec says to use the entry settings object as the base URL, while we're using the location's window object's URL. We should be using the entry_global API instead.

Eijebong added a commit to Eijebong/servo that referenced this issue Oct 11, 2018
Now the /html/browsers/browsing-the-web/navigating-across-documents/005.htm
test is itermittent too for the same reason as 006.htm
See servo#21382

Fixes servo#20906
Eijebong added a commit to Eijebong/servo that referenced this issue Oct 11, 2018
Now the /html/browsers/browsing-the-web/navigating-across-documents/005.htm
test is itermittent too for the same reason as 006.htm
See servo#21382

Fixes servo#20906
bors-servo added a commit that referenced this issue Oct 12, 2018
Use the global entry url in Location::SetHref

Now the /html/browsers/browsing-the-web/navigating-across-documents/005.htm
test is itermittent too for the same reason as 006.htm
See #21382

Fixes #20906

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21926)
<!-- Reviewable:end -->
@jdm
Copy link
Member

@jdm jdm commented Nov 25, 2019

#21926 was an attempt to fix this that hit some surprising test failures that need to be investigated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.