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
morebits.date: Fix bug parsing parens around UTC timestamps #921
Merged
Amorymeltzer
merged 1 commit into
wikimedia-gadgets:master
from
Amorymeltzer:morebits-fixutcbug
Apr 21, 2020
Merged
morebits.date: Fix bug parsing parens around UTC timestamps #921
Amorymeltzer
merged 1 commit into
wikimedia-gadgets:master
from
Amorymeltzer:morebits-fixutcbug
Apr 21, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not sure how this was missed in wikimedia-gadgets#814, but js date will see the parentheses in `(UTC)` and completely ignore the whole thing, setting the timezone local. Should cause issues anywhere, but particularly problematic for the warn module. Vaguely related discussion: https://github.com/azatoth/twinkle/pull/814/files#r374359937
Amorymeltzer
commented
Apr 13, 2020
// Try again after removing a comma, to get MediaWiki timestamps to parse | ||
this._d = new (Function.prototype.bind.call(Date, Date, args[0].replace(/(\d\d:\d\d),/, '$1'))); | ||
// Try again after removing a comma and paren-wrapped timezone, to get MediaWiki timestamps to parse | ||
this._d = new (Function.prototype.bind.call(Date, Date, args[0].replace(/(\d\d:\d\d),/, '$1').replace(/\(UTC\)/, 'UTC'))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
this._d = new (Function.prototype.bind.call(Date, Date, args[0].replace(/(\d\d:\d\d),/, '$1').replace(/\(UTC\)/, 'UTC'))); | |
this._d = new (Function.prototype.bind.call(Date, Date, args[0].replace(/(\d\d:\d\d),/, '$1').replace(/\(UTC\)$/, 'UTC'))); |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably unnecessary, if it ever came up, it wouldn't be detected as a date anyway.
Amorymeltzer
changed the title
morebits: Fix bug parsing parens around UTC timestamps
morebits.date: Fix bug parsing parens around UTC timestamps
Apr 13, 2020
Amorymeltzer
added a commit
to Amorymeltzer/twinkle
that referenced
this pull request
May 3, 2020
See https://en.wikipedia.org/w/index.php?title=Wikipedia_talk:Twinkle&oldid=954707038 As noted in wikimedia-gadgets#921, mediawiki on enwiki uses dates formatted like `19:37, 3 May 2020 (UTC)`, which is an invalid date for two reasons. The first is that (UTC±offset) isn't correct for a timezone, and the native Date converts it to a local date; that is, 17:00 (UTC) become 17:00 (local). The second is that the comma after the time is just plain wrong. Twinkle's new date function handles both of those, except that apparently Firefox 75 will actually now accept the comma as valid. Thus, Firefox was short-circuiting our initial check to see if we had a valid date, before we could clean up mediawiki's parenthesis crap, which was leading to local timezone issues.
wiki-ST47
pushed a commit
to wiki-ST47/twinkle
that referenced
this pull request
Sep 2, 2020
See https://en.wikipedia.org/w/index.php?title=Wikipedia_talk:Twinkle&oldid=954707038 As noted in wikimedia-gadgets#921, mediawiki on enwiki uses dates formatted like `19:37, 3 May 2020 (UTC)`, which is an invalid date for two reasons. The first is that (UTC±offset) isn't correct for a timezone, and the native Date converts it to a local date; that is, 17:00 (UTC) become 17:00 (local). The second is that the comma after the time is just plain wrong. Twinkle's new date function handles both of those, except that apparently Firefox 75 will actually now accept the comma as valid. Thus, Firefox was short-circuiting our initial check to see if we had a valid date, before we could clean up mediawiki's parenthesis crap, which was leading to local timezone issues.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not sure how this was missed in #814, but js date will see the parentheses in
(UTC)
and completely ignore the whole thing, setting the timezone local. Should cause issues anywhere, but particularly problematic for the warn module.Vaguely related discussion: https://github.com/azatoth/twinkle/pull/814/files#r374359937