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

Merge URLUtils into Location #221

Merged
merged 1 commit into from
Oct 6, 2015
Merged

Merge URLUtils into Location #221

merged 1 commit into from
Oct 6, 2015

Conversation

annevk
Copy link
Member

@annevk annevk commented Oct 1, 2015

This fixes the HTML standard side of these issues:

There is still some cleanup work left on the URL side, e.g., making
sure the basic URL parser returns failure for certain conditions when
override state is scheme start state.

@@ -80958,6 +80967,317 @@ State: <OUTPUT NAME=I>1</OUTPUT> <INPUT VALUE="Increment" TYPE=BUTTON O
<p>The <i>relevant <code>Document</code></i> is the <code>Location</code> object's associated
<code>Document</code> object's <span>browsing context</span>'s <span>active document</span>.</p>

<p>The <code>Location</code> object's <i>url</i> is the <i>relevant <code>Document</code></i>'s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#219 used a <dfn>, not an <i>, to associate an internal slot.

@annevk
Copy link
Member Author

annevk commented Oct 2, 2015

I think I addressed all feedback.


<dt><var>location</var> . <code subdfn data-x="dom-hyperlink-search">search</code></dt>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for search and hash it would be especially valuable to point out the preceding ? or #.

@domenic
Copy link
Member

domenic commented Oct 2, 2015

One of these two PRs should remove the reference to URLUtils in the references section. Maybe that's a follow-up commit that cleans up that section in general, e.g. adding data-x-href as well for those terms missing it.

@annevk
Copy link
Member Author

annevk commented Oct 4, 2015

I would like to merge a/area first. Then remove all the redundant things with this PR, such as 32a7a20.

annevk referenced this pull request Oct 4, 2015
The URLUtils abstraction is not working out. See #164 and whatwg/url#62 for details.
@annevk
Copy link
Member Author

annevk commented Oct 6, 2015

I think this is ready now.

@@ -1142,8 +1142,8 @@
ways:</p>

<pre>var a = <span data-x="Document">document</span>.<span data-x="dom-document-links">links</span>[0]; // obtain the first link in the document
a.<span data-x="dom-url-href">href</span> = 'sample.html'; // change the destination URL of the link
a.<span data-x="dom-url-protocol">protocol</span> = 'https'; // change just the scheme part of the URL
a.<span data-x="dom-hyperlink-href">href</span> = 'sample.html'; // change the destination URL of the link
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these actually link to the setter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setter? That doesn't have its own <dfn>.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it would be nice if it did, but probably too big of a departure from the rest of the spec.

@domenic
Copy link
Member

domenic commented Oct 6, 2015

LGTM when the missing "must" is fixed. Would be good to look into setter vs. getter links and the bug links in the sidebar, but not necessary.

@domenic domenic assigned annevk and unassigned domenic Oct 6, 2015
This fixes the HTML standard side of these issues:

* whatwg/url#61
* whatwg/url#62

There is still some cleanup work left on the URL side, e.g., making
sure the basic URL parser returns failure for certain conditions when
override state is scheme start state.
@annevk annevk merged commit f0a7365 into master Oct 6, 2015
@annevk annevk deleted the location branch October 6, 2015 18:04
nathaneliason pushed a commit to nathaneliason/html that referenced this pull request Jun 16, 2022
This is a series of improvements that will help pave the way for a better CI build/deploy architecture in the future, potentially based on GitHub Actions. In particular:

* Stop grabbing pdfsizeopt using git from outside the container. Instead, get it as part of the Dockerfile along with other dependencies. Closes whatwg#221.

* Avoid the multi-step PDF generation process, where we deploy to whatwg.org, point Prince at that, and then deploy the generated print.pdf to whatwg.org. Instead, we serve the built output inside the Docker container, use that to generate print.pdf, and deploy everything to whatwg.org in a single step.

* Update Prince from v11.3 to v13.5.

* Download only the .jar file for the validator, instead of the full package; we can let Docker handle installing Java.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants