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

Addressing Anssi's feedback, expanding the noted items #43

Merged
merged 14 commits into from Jan 22, 2015

Conversation

drott
Copy link

@drott drott commented Jan 21, 2015

  • APIs for interacting with Presentation.. => "API"
  • opening browsing context
  • close, send => own algorithms
  • send TODO moved
  • session property open-issue, still required?
  • AvailableChange event reorg'ed
  • Idioms => uppercase
  • Definition of "presentation"
  • session identifier => add "alphanumeric"
  • Availability updating algorithm isolated
  • Organizing section of onavailable change
    ** Define monitoring algorithm
    ** Define when to start and cancel monitoring
  • Event handlers wording
  • Replace "opener page" => opening browsing context
  • removed .XXX style
  • Padding before headings

anssiko and others added 12 commits January 19, 2015 11:18
* Highlight missing prose and add placeholder text
* Misc editorial changes
(Sorry for the monolithic commit)
* PresentationSession
* opening browsing context
* close, send, own algorithms
* session property open-issue, still required?
* AvailableChange event
* Definition of "presentation"
* session identifier => alphanumeric
** Define monitoring algorithm
** Define when to start and cancel monitoring
@drott
Copy link
Author

drott commented Jan 21, 2015

<h3>
<code>NavigatorPresentation</code>
Common Idioms
Copy link
Member

Choose a reason for hiding this comment

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

Lowercase "idioms" for consistency :-)

@anssiko
Copy link
Member

anssiko commented Jan 21, 2015

LGTM.

* Sentence case for two headers
@mfoltzgoogle
Copy link
Contributor

Overview.src.html:

L669 Instead of ‘requesting page’, can we say

The opening browsing context can then communicate with the presentation as if the user had manually connected to it via startSession.

or just

The page can then communicate with the presentation as if the user had manually connected to it via startSession.

L787 I might say:

A presentation is an active connection between a user agent and a presentation display for displaying web content on the latter at the request of the former.

L793 I might say

A presentation session is an object relating an opening browsing context to its presentation display and enabling two-messaging between them. Each object has a presentation session state and a presentation session identifier to distinguish its session from other presentation sessions.

L801 I might say:

An opening browsing context is a browsing context that has initiated or resumed a presentation session by calling startSession or joinSession.

L986

(drott) What would be a good definition of the session attribute? Do we want to keep it? It's somewhat hard to define, when it should contain a value, and when it should be empty - and it's not compatible with multiple calls to startSession, for example.

The purpose of the session attribute was to provide the presented page with access to the PresentationSession without allowing a potential race condition between the registration of an event handler and the firing of an event, or complicating the browser implementation to fix this case. It should be null in an opening context and non-null in a presented document.

I think we should reconsider this design in light of our use of Promises in other parts of the spec, as well as our intention to allow multiple opener pages to communicate with a given presentation.

L1343 I feel this sentence (or some improved version) should be put in the overview (as modified below) to replace “In the following...” to try to tie together the three algorithms. But I don’t feel strongly.

The user agent must keep a list of available presentation displays. According to the number of event handlers for onavailablechange, the user agent must also keep the list up to date by running the steps in Monitoring the list of available presentation displays.

@mfoltzgoogle
Copy link
Contributor

For the last suggestion, I was specifically suggesting the text be inserted under heading 6.4.4 "The onavailablechange EventHandler."

> L669 Instead of ‘requesting page’, can we say
>> The opening browsing context can then communicate with the presentation as if the user had manually connected to it via startSession.

Done.

> L787 I might say:
>> A presentation is an active connection between a user agent and a presentation display for displaying web content on the latter at the request of the former.

Done.

> L793 I might say
>> A presentation session is an object relating an opening browsing context to its presentation display and enabling two-messaging between them. Each object has a presentation session state and a presentation session identifier to distinguish its session from other presentation sessions.

Added, with two modifications: two-messaging => two-way-messaging, to distinguish its session from... => to distinguish it from...

> L801 I might say:
>> An opening browsing context is a browsing context that has initiated or resumed a presentation session by calling startSession or joinSession.

Good rewording, done.

L986
>> (drott) What would be a good definition of the session attribute? Do we want to keep it? It's somewhat hard to define, when it should contain a value, and when it should be empty - and it's not compatible with multiple calls to startSession, for example.
> The purpose of the session attribute was to provide the presented page with access to the PresentationSession without allowing a potential race condition between the registration of an event handler and the firing of an event, or complicating the browser implementation to fix this case. It should be null in an opening context and non-null in a presented document.

Right, I forgot that this one was mainly meant for the presented page side. I turned my question into a TODO to define this for the presented page side.

> L1343 I feel this sentence (or some improved version) should be put in the overview (as modified below) to replace “In the following...” to try to tie together the three algorithms. But I don’t feel strongly.

Done, moved it up to the beginning of the section. Please check if this resembles what you had in mind.
drott pushed a commit that referenced this pull request Jan 22, 2015
Addressing Anssi's feedback, expanding the noted items, incorporated suggestions from Mark's review. Thanks everyone.
@drott drott merged commit 383acf9 into w3c:gh-pages Jan 22, 2015
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.

None yet

3 participants