Commit graph

912 commits

Author SHA1 Message Date
jenkins-bot 4da9fdc80a Merge "Make comment markers inline-block to fix comment wrapping in Safari" 2022-04-01 15:44:16 +00:00
Bartosz Dziewoński b00b7cd5b5 debughighlighter: Fix date highlighting
Follow-up to 4c29304484.

Change-Id: Id088465c3060bcceb2c2b51d784fa60b9005e867
2022-03-29 23:52:00 +02:00
Ed Sanders 48fdcf1056 Remove data-mw-comment-name attribute from subscribe links
The HeadingItem's name is now present in the DOM, so just traverse
to it and use that instead.

Change-Id: If28e1588742513d606e3d8fcfb259b85acc0a873
2022-03-28 22:30:59 +01:00
jenkins-bot 63c6aceb2c Merge "Implement getTimestampString on CommentItem" 2022-03-26 21:25:13 +00:00
Ed Sanders 215695ad2c Refactor topic subscription logic
This will make it easier to support subscription buttons with
visual enhancements enabled.

Change-Id: I3614eada32885216358143a0cacf65966381679e
2022-03-25 11:57:58 +00:00
Ed Sanders 579b8bb1d4 Implement getTimestampString on CommentItem
Change-Id: I1768e9993debe904d6a228942ad0188486d65c0b
2022-03-24 16:49:35 +00:00
Bartosz Dziewoński c7723baf72 CommentParser: Replace uses of Title with TitleValue
Another small step towards removing the reliance on global state.

Change-Id: Ifb4a5bcbef6606d02f1c7aa7385d72822cb0bad0
2022-03-18 18:24:34 +00:00
Ed Sanders b4b6ae4e81 Make comment markers inline-block to fix comment wrapping in Safari
Bug: T298371
Change-Id: I40da5272fd9c44a5a81e303349d0e8fc404e344d
2022-03-18 14:31:36 +00:00
Ed Sanders 3c66f066c2 Refactor highlights into a class, and add window resize listener
Bug: T303616
Change-Id: Id369cdfc511a7f0f5e19fff4c6424bec2f34ff55
2022-03-14 14:57:19 +00:00
Bartosz Dziewoński 01b253c5b6 Don't allow the root node to be treated like a comment frame
Also fix a bug where headings would be ignored while checking for
comment frames. See task for detailed explanation.

Bug: T303396
Change-Id: I6495826b4b050ea80680e0798ac6ab4497a7c09e
2022-03-10 17:45:08 +00:00
jenkins-bot dd24b0edcd Merge "Improve handling for comments after fake headings using wikitext ;" 2022-03-10 16:21:18 +00:00
jenkins-bot 32d9ef573a Merge "CommentParser: Avoid using a dynamic undeclared property" 2022-03-10 00:22:16 +00:00
jenkins-bot 76478dda26 Merge "Move signatureScanLimit to a constant in JS" 2022-03-10 00:22:14 +00:00
jenkins-bot dfcb2f4fc7 Merge "Fix highlighting of comments when reloading" 2022-03-09 22:44:48 +00:00
Ed Sanders 457f6eb7d1 Fix highlighting of comments when reloading
The CommentController stores a list of the new comments that
triggered the reload warning. Just highlight all the comments
in this list, as we always reload to the revision we got from
the same API call.

Bug: T303261
Change-Id: I960f3cf33e25004bad6ab6df907121a87555aaf4
2022-03-09 14:47:03 +00:00
Bartosz Dziewoński 4c29304484 CommentParser: Avoid using a dynamic undeclared property
Change-Id: Iefa8dea83bc0d31b9c6b3509189eeaa652dd9ea0
2022-03-08 23:30:11 +00:00
Bartosz Dziewoński 08c79142fb ImmutableRange: Add @property annotations for magic props
Phan can analyze them now and reports some issues with types.

* Add some assertions on types where we're sure that we're using an
  Element or non-null, but Phan can't prove it
* Fix incorrect type hints on getFullyCoveredSiblings() and
  getCoveredSiblings(), luckily it was harmless

Change-Id: I8cc12450378efa7434c4d66882378b715edd4a70
2022-03-08 23:29:40 +00:00
Bartosz Dziewoński eb1fe7a8fb CommentParser: Fix redundant uses of getHeadlineNodeAndOffset()
We call CommentUtils::getHeadlineNodeAndOffset() before constructing
the HeadingItem in CommentParser, so the range's startContainer
is always the headline node.

Change-Id: I2afb6ba9100e785cd91f31d82f4cea59fa8b5443
2022-03-08 23:29:34 +00:00
Bartosz Dziewoński 0e576216b2 CommentUtils: Fix confusing types in getIndentLevel()
Change-Id: I548cf4ad54e92c22da64caf53ee028a906cd3b62
2022-03-08 23:29:15 +00:00
Bartosz Dziewoński 584f6a020c Use tagName rather than nodeName when we know the node is an element
`tagName` is only defined on Element, and it returns its tag name.

`nodeName` is defined on Node, and it returns the tag name for Elements,
and a string like '#text' or '#document-fragment' for other types.

We were using both, which made it harder to reason about what types
we're dealing with.

Change-Id: I8e621e5872bdf78c84ec553cfbfcdbf0192f0589
2022-03-08 23:29:05 +00:00
jenkins-bot 1f7ff387a7 Merge "New topic: skip tabbing into the mode tabs until you've focused the body" 2022-03-08 23:18:37 +00:00
jenkins-bot 4b2f9ede8e Merge "Use MessageWidget's showClose option" 2022-03-08 22:41:39 +00:00
Ed Sanders 6869112aad Use MessageWidget's showClose option
Change-Id: I81292bd84ce6feefdb852f6a636109f68e55291d
2022-03-08 01:25:22 +00:00
Bartosz Dziewoński b2ee19b441 Remove check for CDATA nodes
Added in 76289cdf73,
should no longer be needed since we switch to Parsoid's
HTML parser in 3e6ab2c4d2.

Change-Id: Ic0b7ed8089b71f2338e604f68d547759e069f0b2
2022-03-04 22:14:41 +01:00
Ed Sanders 4f91a0b883 Highlight new comments when refreshing
Bug: T300215
Change-Id: I72655e05dccbe96f9dfa4ccca4e91b3edf133d97
2022-03-04 00:40:29 +00:00
Ed Sanders 487be9e202 Show error message as soon as we detect the parent comment has been deleted
Bug: T300504
Change-Id: I654422f7931f50503e51500508aea728adf327a1
2022-03-04 00:40:29 +00:00
Ed Sanders d619308098 Don't show auto-save notifications when dynamically updating the page
Bug: T302327
Depends-On: If94e603458a385ba6eb15c4e29144f72e3ad12ca
Change-Id: I28f965137b986d00ac7a020ca93d8dc5195f5d4f
2022-03-04 00:40:29 +00:00
Ed Sanders da49448f43 Poll for new comments in the section you are replying in
Bug: T300504
Change-Id: I3a887ab2f5260bb4893a3d680103c9d8ec767f45
2022-03-04 00:40:29 +00:00
jenkins-bot 094b77b4bb Merge "Handle reply/topic preview entirely server-side" 2022-03-02 14:13:59 +00:00
Ed Sanders 1032db4878 Move (auto) topic subscription initialisation to separate file
Bug: T302805
Change-Id: Ia901cba731233719dd1d5916eb5211a551d34091
2022-03-01 21:27:08 +00:00
Ed Sanders c70c1e76f3 Only bind page-level handlers once
Raise scope of pageThreads variable (renamed from `result`)
so we don't have to re-bind when it is updated, and use a
flag to ensure we only bind the handlers once.

Bug: T302810
Change-Id: I0a5b122b8e3f656f28f3dd4fa6aca8e7839be06b
2022-03-01 21:26:36 +00:00
Ed Sanders f5d4df12a9 Move highlightPublishedComment to highlighter.js
Bug: T302805
Change-Id: I429148b16f790e1d1f5cbbd84fc35139727b4402
2022-03-01 14:44:21 +00:00
Ed Sanders ecbe75c453 Move highlighting methods to separate file
Bug: T302805
Change-Id: Ie4a8ea3f351015ab5bb4b817ef997cf6d77428da
2022-03-01 14:29:10 +00:00
Ed Sanders b6136c9a58 Rename highlighter to debughighlighter
Change-Id: I8c3873fb345fee3687efb1632f3eae36c030f9cf
2022-03-01 14:20:18 +00:00
jenkins-bot 0dc293fc20 Merge "Warn when target comments in a link can't be found" 2022-02-28 23:24:15 +00:00
jenkins-bot e15ccb8a07 Merge "Highlight all comments since the oldest in a thread bundle" 2022-02-28 23:24:12 +00:00
Bartosz Dziewoński be74938855 Warn when target comments in a link can't be found
Links are generated to:
* Single comments, by a user or by a notification
* Potentially multiple comments (new comments since T),
  usually just by notifications.

If in either case no comments are found to highlight, warn the
user so they get some visual feedback that the notification worked
as expected, but that the comment was just deleted since.

Bug: T301602
Change-Id: I090c904481cf6f3e9f964fa43b10001e75b0bc90
2022-02-28 21:58:59 +00:00
Ed Sanders dc8b4e8d4f Highlight all comments since the oldest in a thread bundle
For topic subscriptions, further restrict this to comments
in the same thread.

Bug: T302014
Change-Id: Ifba218871122901031a891034e709b886fc406da
2022-02-28 21:58:10 +00:00
jenkins-bot e4fa34f025 Merge "Don't insert comment markers inside replaced elements (like <video>)" 2022-02-28 17:16:11 +00:00
jenkins-bot 542da89530 Merge "Don't detect comments within references" 2022-02-28 16:47:21 +00:00
David Lynch 039372e925 New topic: skip tabbing into the mode tabs until you've focused the body
Bug: T295511
Change-Id: I23f964b5d02dc1486e1dcf71c64d337a600fdf9d
2022-02-23 11:24:58 -06:00
Ed Sanders ae554cdd66 Fix legacy hint URI on mobile
Bug: T302307
Change-Id: I7ce370bb011f0b30505c1b195bf6c96317b159e7
2022-02-22 15:31:13 +00:00
Bartosz Dziewoński 1e3ce9c88a Don't insert comment markers inside replaced elements (like <video>)
Also special-case thumbnail wrappers generated by
MediaTransformOutput::linkWrap, for compatibility with
TimedMediaHandler.

Bug: T301427
Bug: T302296
Change-Id: I7f48d8b2261507c5a33526c54109f5187d062ed3
2022-02-22 15:11:34 +00:00
Bartosz Dziewoński 8a2715bdd5 Move signatureScanLimit to a constant in JS
Change-Id: Ieb60c148fd060ab62e4a493e2d0dff6c051f945c
2022-02-21 22:42:14 +01:00
Bartosz Dziewoński 4244418e56 Don't detect comments within references
Bug: T301213
Change-Id: Ifd5198651c8ed0ce53379fb5e35938089cd54a09
2022-02-21 19:57:44 +00:00
Bartosz Dziewoński 1d43a024f9 Handle reply/topic preview entirely server-side
We were rendering the preview in a completely different way from how
we would add the real reply, and the results would be different
sometimes, particularly for multi-line comments with messed-up markup.

Render it server-side instead, in a very similar way to real replies
(generating a DOM list node and transforming it through Parsoid),
although without the whole context of the page to improve performance.

We can remove a lot of client-side code that was used solely for this.

This will allow the preview to accurately display the signatures when
we change how they are added (T278442), without us having to implement
those changes again from scratch for the preview.

Change-Id: I53341f4d4075c25b67ec3b3032bff9b8a880dcd3
2022-02-21 17:42:28 +01:00
Bartosz Dziewoński 8e44b43df0 Split off ThreadItemSet from CommentParser
Goal:
-----
Finishing the work from Iadb7757debe000025e52770ca51ebcf24ca8ee66
by changing CommentParser::parse() to return a data object, instead of
the whole parser.

Changes:
--------
ThreadItemSet.php:
ThreadItemSet.js:
* New data class to access the results of parsing a discussion. Most
  methods and properties are moved from CommentParser with no changes.

CommentParser.php:
Parser.js:
* parse() returns a new ThreadItemSet.
* Remove methods moved to ThreadItemSet.
* Placeholder headings are generated slightly differently, as we process
  things in a different order.
* Grouping threads and computing IDs/names is no longer lazy. We always
  needed IDs/names anyway.
* computeId() explicitly uses a ThreadItemSet to check the existing IDs
  when de-duplicating.

controller.js:
* Move the code for turning some nodes annotated by CommentFormatter
  into a ThreadItemSet (previously a Parser) from controller#init to
  ThreadItemSet.static.newFromAnnotatedNodes, and rewrite it to handle
  assigning parents/replies and recalculating legacy IDs more nicely.
* mw.dt.pageThreads is now a ThreadItemSet.

Change-Id: I49bfe019aa460651447fd383f73eafa9d7180a92
2022-02-21 16:22:32 +00:00
Bartosz Dziewoński 4613ae78e7 Change CommentParser into a service
Goal:
-----
To have a method like CommentParser::parse(), which just takes a node
to parse and a title and returns plain data, so that we don't need to
keep track of the config to construct a CommentParser object (the
required config like content language is provided by services) and
we don't need to keep that object around after parsing.

Changes:
--------
CommentParser.php:
* …is now a service. Constructor only takes services as arguments.
  The node and title are passed to a new parse() method.
* parse() should return plain data, but I split this part to a separate
  patch for ease of review: I49bfe019aa460651447fd383f73eafa9d7180a92.
* CommentParser still cheats and accesses global state in a few places,
  e.g. calling Title::makeTitleSafe or CommentUtils::getTitleFromUrl,
  so we can't turn its tests into true unit tests. This work is left
  for future commits.

LanguageData.php:
* …is now a service, instead of a static class.

Parser.js:
* …is not a real service, but it's changed to behave in a similar way.
  Constructor takes only the required config as argument,
  and node and title are instead passed to a new parse() method.

CommentParserTest.php:
parser.test.js:
* Can be simplified, now that we don't need a useless node and title
  to test internal methods that don't use them.

testUtils.js:
* Can be simplified, now that we don't need to override internal
  ResourceLoader stuff just to change the parser config.

Change-Id: Iadb7757debe000025e52770ca51ebcf24ca8ee66
2022-02-19 19:51:57 +01:00
Bartosz Dziewoński 99b5de8038 Split Data class into ResourceLoaderData and LanguageData
The Data class contained utilities for two unrelated purposes.
Split each half to a separate class.

Notably, this improves the signature of the getLocalData() function.

Change-Id: Icde615fb9d483fee1f352c34909b37f8ffde8081
2022-02-19 19:37:34 +01:00
Bartosz Dziewoński ae9f26a9e5 Various code quality tweaks
(suggested by PhpStorm)

composer.json:
* Document required PHP extensions

Parser.js:
* Remove incorrect param documentation
* Fix some typos in comments (missing parentheses)

CommentParser.php:
* Fix some typos in comments (missing parentheses)

ImmutableRange.php:
* Remove unused property
* Add a `throw` to indicate that code path is unreachable

SubscribedNewCommentPresentationModel.php:
* Add missing `return false`

CommentParserTest.php:
* Remove unnecessary pass-by-reference

CommentModifierTest.php:
* Remove unused variable

CommentParserTest.php:
* Don't construct Element objects directly. PHP's DOMElement allows
  it, but Parsoid/Dodo's doesn't, and we use the latter for static
  analysis. This generates all kinds of confusing warnings.

Change-Id: Ia9598ebea0e99830dd485296e94a9d96acc4b258
2022-02-19 19:36:52 +01:00