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

Delete DOMTimestamp and EpochTimestamp #17096

Merged
merged 20 commits into from
Nov 15, 2022
Merged

Conversation

teoli2003
Copy link
Contributor

These are not real interfaces but a kind of WebIDL-only typedef. We wanted to remove them for a long time.

This PR adapts all occurrences of DOMTimestamp and EpochTimestamp.

This is part of the cleaning needed for having the whole of Web/API covered with page-type: YAML headers. (See #16255)

@teoli2003 teoli2003 requested a review from wbamberg June 8, 2022 06:43
@teoli2003 teoli2003 requested review from a team as code owners June 8, 2022 06:43
@teoli2003 teoli2003 requested review from a team and Guyzeroth and removed request for a team June 8, 2022 06:43
@github-actions github-actions bot added Content:Other Any docs not covered by another "Content:" label Content:WebAPI Web API docs labels Jun 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2022

Preview URLs (17 pages)
Flaws (53)

Note! 11 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/Document_Object_Model
Title: Document Object Model (DOM)
Flaw count: 45

  • macros:
    • /en-US/docs/Web/API/NodeFilter redirects to /en-US/docs/Web/API/Document/createNodeIterator
    • /en-US/docs/Web/API/SVGFilterPrimitiveStandardAttributes does not exist
    • /en-US/docs/Web/API/SVGHatchElement does not exist
    • /en-US/docs/Web/API/SVGHatchpathElement does not exist
    • /en-US/docs/Web/API/SVGColor does not exist
    • and 40 more flaws omitted

URL: /en-US/docs/Web/API/RTCPeerConnection/generateCertificate
Title: RTCPeerConnection.generateCertificate() static function
Flaw count: 4

  • macros:
    • /en-US/docs/Web/API/AlgorithmIdentifier does not exist
    • /en-US/docs/Web/API/Algorithm does not exist
    • /en-US/docs/Web/API/RTCCertificateExpiration does not exist
    • /en-US/docs/Web/API/Algorithm does not exist

URL: /en-US/docs/Web/API/CookieStore/set
Title: CookieStore.set()
Flaw count: 1

  • macros:
    • /en-US/docs/Glossary/serialize redirects to /en-US/docs/Glossary/Serialization

URL: /en-US/docs/Web/API/Geolocation_API/Using_the_Geolocation_API
Title: Using the Geolocation API
Flaw count: 1

  • macros:
    • The fourth to sixth parameters of 'EmbedLiveSample' are deprecated

URL: /en-US/docs/Mozilla/Firefox/Releases/54
Title: Firefox 54 for developers
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/WorkerGlobalScope/close redirects to /en-US/docs/Web/API/DedicatedWorkerGlobalScope/close

URL: /en-US/docs/Mozilla/Firefox/Releases/23
Title: Firefox 23 for developers
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLMediaElement/initialTime does not exist
External URLs (2)

URL: /en-US/docs/Glossary/Unix_time
Title: Unix time

(this comment was updated 2022-11-15 01:08:40.831579)

@teoli2003 teoli2003 requested review from schalkneethling and a team as code owners June 27, 2022 10:09
@teoli2003 teoli2003 requested a review from a team June 27, 2022 10:09
@teoli2003 teoli2003 requested review from a team as code owners June 27, 2022 10:09
@teoli2003 teoli2003 requested review from dipikabh and willdurand and removed request for a team June 27, 2022 10:09
@github-actions github-actions bot added Content:Accessibility Accessibility docs Content:CSS Cascading Style Sheets docs Content:Glossary Glossary entries labels Jun 27, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

This pull request has merge conflicts that must be resolved before it can be merged.

@hamishwillee
Copy link
Collaborator

@teoli2003 This needs a rebase and remaining fixes. Would be great to get in.

@wbamberg
Copy link
Collaborator

wbamberg commented Nov 8, 2022

@hamishwillee , I think @Elchi3 and I are going to have to take care of this anyway as part of the work on the Performance API: openwebdocs/project#62.

@Elchi3
Copy link
Member

Elchi3 commented Nov 11, 2022

Many typedefs probably need no page in the Web/API/ reference. Some (like these two) probably need a glossary page or some docs somewhere. It would be nice to have a plan about typedefs generally. I opened a discussion: mdn/mdn-community#282

@hamishwillee
Copy link
Collaborator

@wbamberg Re https://github.com/orgs/mdn/discussions/282#discussioncomment-4141217

other specs - it is generally a unix epoch timestamp, but it doesn't have to be.

This I don't understand though, I thought DOMTimeStamp was replaced with EpochTimeStamp (according to https://developer.mozilla.org/en-US/docs/Web/API/DOMTimeStamp, and also features like https://developer.mozilla.org/en-US/docs/Web/API/Notification/timestamp, which we document as using DOMTimeStamp, but the spec uses EpochTimeStamp: https://notifications.spec.whatwg.org/#timestamp).

See the note in DOMTimeStamp:

image

So yes, it has been removed and renamed in one spec, but it still gets reference in others. Lazy Hamish says it probably doesn't matter all that much - ie. those other specs are not all that important. But I have not done the investigation.

@wbamberg
Copy link
Collaborator

wbamberg commented Nov 15, 2022

I've fixed the merge conflicts here and made the changes I recommended in my own review :). So this PR now does three things:

  • adds a new glossary entry "Unix time"
  • deletes the pages for DOMTimeStamp and EpochTimeStamp, and made them redirect to the glossary entry
  • changes all links to DOMTimeStamp and EpochTimeStamp to links to the glossary entry

After this there are no references to DOMTimeStamp or EpochTimeStamp on MDN.

This doesn't yet tackle DOMHighResTimeStamp - I think that does not belong in a "Unix time" glossary entry, but in a bit of guide material for the Perf APIs, and it will be cleaner to do it in a different PR. When we do that, we should update the glossary entry to point lost people at this new guide material, like "if you are looking for high-res (sub-millisecond) timestamps used in the perf APIs, see XYZ".

@wbamberg wbamberg dismissed their stale review November 15, 2022 00:06

I added my own recommendations!

Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
the `GeolocationCoordinates` object is part of the
{{domxref("GeolocationPosition")}} interface, which is the object type returned by
Geolocation API functions that obtain and return a geographical position.
The {{domxref("GeolocationCoordinates")}} interface's read-only **`longitude`** property is a double-precision floating point value which represents the longitude of a geographical position, specified in decimal degrees.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps not relevant to this change, but does it matter that it is double-precision floating point? Isn't it just a number in JavaScript

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Excellent. Just a couple of comments. If you either respond or mark those resolved I am happy to merge (or for you to).

notification is actual. For example, this could be in the past when a notification
is used for a message that couldn't immediately be delivered because the device
was offline, or in the future for a meeting that is about to start.
- : A timestamp, given as [Unix time](/en-US/docs/Glossary/Unix_time) in milliseconds, representing the time associated with the notification. This could be in the past when a notification is used for a message that couldn't immediately be delivered because the device was offline, or in the future for a meeting that is about to start.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought "the time at which a notification is actual." was a typo but it is a quote from the spec: https://notifications.spec.whatwg.org/#timestamp. Maybe we should keep it? I don't love "associated with" either.

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW I am OK with this and would have complained about "actual".

@wbamberg wbamberg merged commit e853d95 into mdn:main Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Awaiting for author to address review/feedback Content:Glossary Glossary entries Content:Other Any docs not covered by another "Content:" label Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants