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

Remove infoset #359

Merged
merged 7 commits into from
Nov 5, 2018
Merged

Remove infoset #359

merged 7 commits into from
Nov 5, 2018

Conversation

mattgarrish
Copy link
Member

This PR removes the "infoset" concept and replaces it with more emphasis on the authored v. canonical manifests.

Prior to this, we had infoset and manifest subsections for almost every property, whereas now the information has been merged and, in most cases, the manifest expressions moved up after the introductory paragraph.

As this change required a significant reread of the specification, I've also made a number of minor editorial tweaks along the way. These are generally insignificant, with the exception of the language and direction section. As I read through, I noticed that most of the global language info was specified before the subsection containing the property definitions (plus at least one para that looked like it belonged with the overrides). @iherman , please have a look and see if my shuffling still makes sense to you.

Otherwise, I think there's a diagram or two that will need updating, but I didn't try and fix those.

mattgarrish and others added 4 commits October 31, 2018 11:25
- I made changes in the new section on authored vs. canonical manifest
- in the case of embedded manifest's base URL I have added reference to the pending issues, and also removed the reference to xml:base (it has been removed from the latest HTML versions)
various editorial tweaks noticed along the way
@mattgarrish
Copy link
Member Author

Not sure why no preview option is showing up. Only thing I can think is because the diff is too large. Respec doesn't show me any issues.

@iherman
Copy link
Member

iherman commented Nov 1, 2018

I do not know why preview does not work either.

This URI seems to work to display the new version:

https://tinyurl.com/y9ds858k

@iherman
Copy link
Member

iherman commented Nov 1, 2018

I have committed the document with some minor changes.

Copy link
Member

@iherman iherman left a comment

Choose a reason for hiding this comment

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

I have a problem with Section 3.1.1. It can be a bit misleading: it is as if the authored and the canonical manifests were disjoint categories. But they are not: an author may very well choose to create a canonical manifest directly; this is all the more true if "author" is not a human but some computer program. A canonical manifest is a perfectly valid authored manifest, though that is not what the section suggests.

Maybe the problem lies with the term "authored". What about "simple", "human centric" (none of these are good:-(

@mattgarrish
Copy link
Member Author

Yes, I had the same thought last night, but even if authored=canonical, you don't actually get canonical until the user agent has processed the authored manifest, correct? That was the distinction I was trying to draw, between the one that someone/something has produced versus the post-processed.

Would it help to add some more clarification to that effect?

Unknown accessibility terms should not be removed, just a warning issued.
@iherman
Copy link
Member

iherman commented Nov 1, 2018

Well, if authored=canonical then the canonicalization is an identity operation... But yes, it will still happen, in this sense it is not 100% the same.

I think adding a somewhat more verbose clarification would indeed be the best, rather than producing a concise and fully precise text...

@mattgarrish
Copy link
Member Author

I added another para, but feel free to mod directly if you have other ideas.

@iherman
Copy link
Member

iherman commented Nov 1, 2018

Ok, let us see what others think. It works for me...

@mattgarrish
Copy link
Member Author

Here's an alternative preview that doesn't break when you click internal links:
https://cdn.staticaly.com/gh/w3c/wpub/remove-infoset/index.html

@mattgarrish
Copy link
Member Author

Should we go ahead and merge this so we don't block the other open issues? (It's mostly shuffling, so if it can benefit from further clarification that shouldn't impact on what exists now.)

@iherman
Copy link
Member

iherman commented Nov 5, 2018

Yes.

@TzviyaSiegman TzviyaSiegman merged commit 8e62b59 into master Nov 5, 2018
@mattgarrish mattgarrish deleted the remove-infoset branch November 5, 2018 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants