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

Use <base> in resolving url attributes (like "href"). #6303

Closed
wants to merge 1 commit into from

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Jun 7, 2015

This allows polymer demos to load. For example, paper-buttons:
image

Running the demo also requires the following userscript:

(function(window) {
    // Work around the fact that this userscript is inserted into
    // *every* single HTML <link rel="import"> document (fragment?).
    if(window.servoPolyfillLoaded)
        return;
    window.servoPolyfillLoaded = true;

    // Stubbed SVG "support".
    var Element = window.Element;

    function SVGElement() {
        Element.apply(this, arguments);
    }
    SVGElement.prototype = Object.create(Element.prototype);
    SVGElement.prototype.constructor = SVGElement;
    SVGElement.prototype.style = {};
    window.SVGElement = SVGElement;

    var createElementNS = document.createElementNS;
    document.createElementNS = function(ns, tag) {
        var e = createElementNS.apply(this, arguments);
        if(ns === 'http://www.w3.org/2000/svg') {
            e.__proto__ = SVGElement.prototype;
        }
        return e;
    };

    // Event bubbling from document to window.
    var dispatchEvent = window.dispatchEvent;
    var Event = window.Event;
    ['DOMContentLoaded', 'HTMLImportsLoaded', 'WebComponentsReady'].forEach(function(type) {
        document.addEventListener(type, function(ev) {
            dispatchEvent.call(window, new Event(type));
        });
    });

    // Hacky style info for ripples.
    window.getComputedStyle = function(e) {
        return {color: e.parentNode.classList.contains('colorful') ? '#4285F4' : 'black'};
    };

    // Servo shouldn't advertise <template> if it's not supported.
    window.HTMLTemplateElement = undefined;

    // Send "load" events to <style> elements as needed.
    addEventListener('DOMContentLoaded', function() {
        setInterval(function triggerLoad() {
            [].forEach.call(this.querySelectorAll('style'), function(e) {
                if(!e.servoStyleLoaded) {
                    e.servoStyleLoaded = true;
                    e.dispatchEvent(new Event('load'));
                }
            });
        }.bind(document), 100);
    });
})(this);

Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 7, 2015
@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/5205

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jun 7, 2015

Let's not run querySelector every time we need to resolve a URL.

@nox
Copy link
Contributor

nox commented Jun 9, 2015

I've discussed with @eddyb about what he should do on IRC. Assigning ticket to myself because I'm curious about reviewing it.

@nox nox self-assigned this Jun 9, 2015
@nox nox added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 9, 2015
@Ms2ger Ms2ger mentioned this pull request Jul 6, 2015
@jdm
Copy link
Member

jdm commented Jul 6, 2015

@nox: Can you add the changes that needed to be made here?

@nox
Copy link
Contributor

nox commented Jul 7, 2015

Will do today or tonight.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 7, 2015

I think I agree with @bzbarsky that we should add a base_url getter to the Document instead.

@bzbarsky
Copy link
Contributor

bzbarsky commented Jul 7, 2015

You need that base_url getter no matter what, because there are cases when a document has no <base> tag and its base URI is not the document URI. See https://html.spec.whatwg.org/multipage/infrastructure.html#fallback-base-url

Given also that resolving URIs happens a good bit during pageload, you want to store the "fallback base URL" directly in the document, and probably store the "document base URL" as well (updating it when <base> elements get added/removed/modified).

@jdm
Copy link
Member

jdm commented Jul 7, 2015

Sorry about the ambiguous wording; I was really asking if the proposed changes mentioned in "I've discussed with @eddyb about what he should do on IRC" could be listed here so it was clear what work still remains. I wasn't actually trying to crack the whip on anybody.

@nox
Copy link
Contributor

nox commented Jul 8, 2015

Needs to look for <base> insertion or removal in the tree through the virtual methods, and store the associated base element in Document in a MutNullableHeap<JS<HTMLBaseElement>> field.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 8, 2015

We should implement fallback_base_url and base_url methods on Document that match https://html.spec.whatwg.org/multipage/infrastructure.html#fallback-base-url.

@metajack
Copy link
Contributor

@nox ping?

@nox
Copy link
Contributor

nox commented Jul 29, 2015

@metajack Will take it over I guess.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 29, 2015

I'm working on a better approach.

@Ms2ger Ms2ger closed this Jul 29, 2015
bors-servo pushed a commit that referenced this pull request Mar 28, 2016
Use <base> in resolving url attributes (like "href").

Second take of #6303, now that the `base_url` infrastructure is in place.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10247)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this pull request Mar 29, 2016
Use <base> in resolving url attributes (like "href").

Second take of #6303, now that the `base_url` infrastructure is in place.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10247)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this pull request Mar 29, 2016
Use <base> in resolving url attributes (like "href").

Second take of #6303, now that the `base_url` infrastructure is in place.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10247)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-code-changes Changes have not yet been made that were requested by a reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants