Frozen base URL definition is broken #1060
Comments
|
This got broken by 30bc2557 looks like. Before that, it used to say: |
|
This whole setup seems broken. As far as I can tell the href IDL attribute reflects the href content attribute, but that is not what the specification says. E.g., per the specification setting it to "https://test:test/" should end up returning the fallback base URL, but browsers will just return the literal value. Now that might not end up being used as base URL of course, but that's something else again. |
|
We can't use reflect however since we have a custom base URL. So, the getter should simply return the element's internal base URL, serialized, if it's non-null, and the href attribute value otherwise, and the setter should set the href attribute. We should also take action upon every href attribute value set, not only when it's the first base element in a document. And make that set the internal base URL as appropriate, taking into account CSP. @mikewest is it fine for CSP to run "Is base allowed for Document?" for each base element? |
|
@annevk: That would potentially end up creating multiple violation reports. Not the end of the world, but it would be better to process things once. |
|
Okay, so we should do that whenever we determine whether the document base URL needs to change. |
|
I didn't test what browsers do about the In terms of what the |
|
Hmm indeed, <base href=/test/>
<script>
var b = document.createElement("base")
w(b.href)
</script>logs the document URL, not the document base URL, in both Firefox and Chrome. |
|
For the failure scenario, <script>
var b = document.createElement("base")
b.href = "//test:test"
w(b.href)
</script>Firefox returns the given value, but Chrome and Safari return the empty string. Don't have Edge. |
<iframe onload="var b=this.contentDocument.createElement('base');b.href='test';w(b.href)"></iframe>returns "test" in Firefox and the empty string in Chrome and Safari. |
|
I also tested OP's assertion that the first <iframe onload="var d=this.contentDocument,b=d.createElement('base');b.href='t';d.head.appendChild(b);w(d.baseURI);w(b.href)"></iframe>It does seem true for <iframe srcdoc="" onload="var d=this.contentDocument,b=d.createElement('base');b.href='t/';d.head.appendChild(b);w(d.baseURI);w(b.href)"></iframe>logs (Use http://software.hixie.ch/utilities/js/live-dom-viewer/ to run these examples for yourself.) So I guess this raises these questions:
|
annevk
added
the
compat
|
Re 4: I noticed the failure case issue in #561 but missed to fix it. Per #561 (comment) there is little interop. I think I prefer returning the input so it's more consistent. |
|
@annevk Before I start looking at the PR, can you explain to me what you're trying to change? Are you changing what the Have you put your testcases somewhere publicly loadable? I'm happy to test IE and Edge for you, but don't really want to try to recreate your testcases in that environment; even loading a URL is pain enough there, much less typing things. |
Safari: Logs null and empty string. What behavior are you aiming for with the PR on these testcases? Oh, and as far as the Firefox behavior here, some notes:
|
|
Apart from the failure scenario (for which the PR matches Safari/Chrome, but should probably be updated to match Firefox per @zcorpan's comment), the PR matches Edge (I'm assuming you have some copypasta and Edge actually logs a trailing slash as well for the final test). Currently the PR also takes into account a "frozen base URL" if the element has one, so that when CSP applies the |
Er, right you are. Sorry about that. I assume the "failure scenario" is http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4080? If so, I'm ok with either the WebKit or Gecko behavior there, I guess; there's no way to win. I agree that having The changes in the PR look good with those two caveats. |
Always parse <base> elements against the document's fallback base URL. When parsing fails, returns their href attribute value. (Note that it can only fail if there is an href attribute value.) This matches the semantics of "reflect", but does not directly use that mechanism since it does not support arbitrary base URLs and such. Fixes #1060. (That issue also has links to tests and results for contemporary browsers.)
|
The PR that landed uses the Gecko handling for failure and does not let CSP have an effect on the |
|
Thank you for reporting this! |
|
I included some web platform tests in the patch I just put up in https://bugzilla.mozilla.org/show_bug.cgi?id=1266077 |
https://html.spec.whatwg.org/multipage/semantics.html#set-the-frozen-base-url step 2 says "Parse the value of element's href content attribute relative to document". This is wrong. It should be relative to the document's fallback base URL. The
hrefgetter defined immediately afterward gets this right...The text was updated successfully, but these errors were encountered: