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

Conform 'current time' definition to the current hr-time spec #236

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

igneel64
Copy link

@igneel64 igneel64 commented Sep 29, 2020

Conform 'current time' definition to the current hr-time spec current high resolution time.

Added HR-TIME-ED localBiblio entry.


Preview | Diff

Conform 'current time' definition to the current hr-time spec

Add HR-TIME-ED localBiblio
the current moment in time.</p>
<p>The term <dfn>current time</dfn> refers to the result of running
<a data-cite="HR-TIME-ED#dfn-current-high-resolution-time">current high
resolution time</a> where |current global| is the {{Window}} object.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can add an inline issue (<p class=issue> IIRC) to align this to the published spec, once we've actually published it

Copy link
Author

Choose a reason for hiding this comment

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

Probably not needed as this will be on L3, right ?

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed!

<p>The term <dfn>current time</dfn> refers to the number of
milliseconds since the start of navigation of the document until
the current moment in time.</p>
<p>The term <dfn>current time</dfn> refers to the result of running
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'd be better off eliminating this dfn, and call the algorithm directly with the current global object from the few places in the processing model that are calling it.

The reason for that is that we need to call it with a specific global object, and this dfn doesn't give us that. Does that make sense?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good and also seems more robust since I have observed that the term current time is mostly used in algorithmic steps in this spec.

So would replacing occurences like:
record the <a>current time</a> in <a>startTime</a>
with
record the result of <a data-cite="HR-TIME-ED#dfn-current-high-resolution-time">current high resolution time</a> where |current global| is {{Window}} in <a>startTime</a>
fit the case ?

Copy link
Contributor

Choose a reason for hiding this comment

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

{{Window}} is a reference to the Window object, but not to a specific window.
You probably want to reference the current global object

@yoavweiss
Copy link
Contributor

@igneel64 - Friendly ping! I'd love to help you land this..

@yoavweiss yoavweiss added this to Triage in Triage May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Triage
Triage
Development

Successfully merging this pull request may close these issues.

None yet

2 participants