Commit graph

876 commits

Author SHA1 Message Date
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
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 4244418e56 Don't detect comments within references
Bug: T301213
Change-Id: Ifd5198651c8ed0ce53379fb5e35938089cd54a09
2022-02-21 19:57:44 +00: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
Bartosz Dziewoński f9f7aeb295 Prevent floated content from leaking out of the preview container
Change-Id: Ic4c5267cbe44fa22537a708f66e08503d0c1f816
2022-02-14 20:47:39 +01:00
Bartosz Dziewoński 4ff65cf5e2 Update references to warningbox class
Bug: T301401
Depends-On: I632e42ee9065b38dca443beae1cb01ef714cae7b
Depends-On: I7a7046bf9b9765cbb82ec3caa1530de7f05e0da4
Change-Id: Ia42cf9cd5b63cfe679bdf90bfcbf528f14f9ebcc
2022-02-09 21:36:50 +01:00
Ed Sanders f0940557ee build: Update eslint-config-wikimedia to 0.22.1
Change-Id: I18a5ceae9e8fdc6398cd27c24c1a97fe87e3c016
2022-02-09 13:50:55 +00:00
jenkins-bot 362206e572 Merge "Improve check for HTML comments after multiple line breaks" 2022-02-08 20:17:26 +00:00
jenkins-bot e8e9ba5f98 Merge "Fix some typos in comments" 2022-02-08 20:00:11 +00:00
jenkins-bot 4593a9fe91 Merge "Don't count leading/trailing whitespace against signature scan limit" 2022-02-08 20:00:08 +00:00
jenkins-bot e82a4d86f3 Merge "Don't clear target comment highlight when Ctrl-clicking" 2022-02-08 19:49:44 +00:00
Bartosz Dziewoński fa01395acc Improve check for HTML comments after multiple line breaks
In the example in T301214, there is a space character between the line
breaks.

Bug: T301214
Change-Id: Ia1a8279ba0c60f92c1a30463c2a39d9e954ae3ca
2022-02-08 01:38:56 +01:00
Ed Sanders 025130fcd2 Don't prompt to reload the page if the reply widget is open
Only show the reload prompt if the 'disappeared' error is detected
when the reply widget is loading. Don't show it if we failed
to save, as it suggests you will not lose content if you reload.

Change-Id: I106f274e180dc97b540d729e31aae575c43f29f0
2022-02-07 22:45:35 +00:00
Ed Sanders 3d0849112a ReplyWidget: Factor out #createErrorMessage
Change-Id: Ie7e34154d74c24ce8379552fdf13feba31fd401f
2022-02-07 22:44:09 +00:00
jenkins-bot 4ac8e94084 Merge "ReplyWidget: Introduce a 'refresh' teardown mode that preserves auto-save" 2022-02-07 21:45:13 +00:00
Bartosz Dziewoński 909f1383d4 Don't clear target comment highlight when Ctrl-clicking
It annoys me when Ctrl-click clears it but middle-click does not.

Change-Id: I8636140df25d44ac79cb2bc0d1aebecc62c4db5d
2022-02-07 21:29:01 +00:00
Ed Sanders 7cad692afa Move ApiDiscussionTools to ApiDiscussionToolsPageInfo
Also remove 'paction' param, and instead make 'transcludedFrom' a
property of the result object.

Change-Id: I0a289f6d71e4708afff0b52066b1ed6faf76b9ae
2022-02-07 16:50:02 +00:00
Bartosz Dziewoński 5945a4a0eb Fix some typos in comments
Change-Id: I699d9d105b8706cef0800ccc086cde687de54078
2022-02-04 20:36:28 +01:00
Bartosz Dziewoński 13ab1db6da Don't count leading/trailing whitespace against signature scan limit
It's an arbitrary limit, it seems harmless to relax it to support the
use case in the task, even if it's weird.

Bug: T300949
Change-Id: I7c895c7019726758bbae3183b9c3ecbd9eabcf38
2022-02-04 19:35:29 +00:00
Ed Sanders 0b42aea276 CommentParser: Cache variables in getUsernameFromLink
Change-Id: I625e6ded3badd75a7a658c8d000576d0d165a18b
2022-02-04 19:35:18 +00:00
Ed Sanders 8ad1df7dc8 CommentParser: Name parts of return value from findSignature
Change-Id: I3a5ad36df0afdedc0aa9a15e5d83c5426b03b790
2022-02-04 19:34:18 +00:00
Ed Sanders e2f3c2f264 ReplyWidget: Introduce a 'refresh' teardown mode that preserves auto-save
Change-Id: Ibb04f7c2813b9d3cf2bb2434a414c86eb0a7f644
2022-02-02 18:38:22 +00:00
jenkins-bot b0b965512e Merge "Create MemoryStorage wrapper around mw.SafeStorage" 2022-02-02 17:27:37 +00:00
jenkins-bot fabe580343 Merge "Limit where whitespaceParsoidHack() is used" 2022-02-02 17:25:02 +00:00
jenkins-bot 11892a9369 Merge "Remove unused code in JS modifier" 2022-02-02 17:21:37 +00:00
Ed Sanders 9e43f5c261 ReplyWidgetVisual: Fully clear sessionStorage when clearing
Clearing the widget removes all the surfaces, meaning there are
no doc-state variables to cleanup. Switch the order of these
two calls.

Change-Id: I6c095a171096cd700ce4cd31b08fa3b982ab2401
2022-02-02 16:46:57 +00:00
Ed Sanders cb5d585b93 Update wgCurRevisionId/wgRevisionId when refreshing the page
Change-Id: I2b1a7a6fe539a8a4bd5e3f865e4bb7464913d9e0
2022-02-02 16:46:57 +00:00
Ed Sanders dd9d37b555 Create MemoryStorage wrapper around mw.SafeStorage
This means we can guarantee we will be able to recover
auto-saves if we don't reload the page.

Change-Id: Ic867ae6df0c949f35cb32ec4b835688778db29ef
2022-02-02 14:59:47 +00:00
Bartosz Dziewoński fb474efb2b controller: Separate updating page contents from saving the page
Bug: T300502
Change-Id: I52c0a648ee15cc61890413a269708a6cb2bd4df8
2022-02-02 00:42:41 +01:00
Ed Sanders 7381d9d963 Cleanup CommentController and document
* Document all methods
* Rename comment to threadItem
* Use this.threadItem instead of passing in identical
  threadItem in various methods.
* Don't pass threadItem to ReplyWidget as we already
  pass the whole CommentController.

Change-Id: If9aad0bcf9f0e4ebf3342b75631ddac8b57f7d87
2022-02-02 00:05:19 +01:00
jenkins-bot 1fdefedc69 Merge "Clear autosave when following links from the legacy hint" 2022-01-31 20:16:21 +00:00
Ed Sanders 6af617ef99 CommentItem: Fix typo in documentation
Change-Id: I95fc9f4ca904a9315fc665618085c6c250b21a2e
2022-01-31 17:01:03 +00:00
Ed Sanders cb828828e2 controller: Fix typo in documentation
Change-Id: Ia68bcac6be0c59146655c5cbce872ea9b9304e37
2022-01-31 15:03:05 +00:00
Bartosz Dziewoński 15f0867b75 Limit where whitespaceParsoidHack() is used
* We don't need it anywhere in JS, since we're not sending that HTML
  to Parsoid.
* We only need it on the nodes directly containing our reply lines in
  PHP, not all over the place.

Change-Id: I0a04388225f32654dda2f599442cd27a303b5d0a
2022-01-29 22:42:46 +00:00
Bartosz Dziewoński bacf6a8fc6 Remove unused code in JS modifier
I don't think we're going to need this in the client again.

Change-Id: Id38405c14edfd7ac45afad8f32cad64594ae7bc0
2022-01-29 21:39:45 +00:00
David Lynch 3fdaf7ee58 Clear autosave when following links from the legacy hint
Bug: T299270
Change-Id: Ife590b4bea359168f07aa308128425cfb46f71cb
2022-01-27 11:48:42 -06:00
Bartosz Dziewoński 110a59200f One more tweak for comparing comment ranges to transclusion/DOM ranges
When we encounter a node that doesn't represent comment contents, e.g.:
* a [reply] link we inserted (T297034#7641334)
* an {{outdent}} template (see changed test case)

…we should ignore it together with its descendants (like in
Parser#nextInterestingLeafNode), instead of processing descendants
and possibly detecting comment contents in them.

Follow-up to 8de940b587,
72b9c2c6f5.

Bug: T297034
Change-Id: Ib2fa40c5fa389572b0e88ef558728fa06e3621b0
2022-01-24 17:42:18 +00:00
Bartosz Dziewoński f15693eefa Use class list everywhere for adding/checking CSS classes
In PHP, use DOMCompat::getClassList(), provided by Parsoid.
In JS, use `.classList`, available in all supported browsers.

This may fix some bugs where we were incorrectly checking for exactly
one class. The change in isOurGeneratedNode() is needed for
Ib2fa40c5fa389572b0e88ef558728fa06e3621b0.

Change-Id: Ia28d31678fd3d617b69280c4b7857755300fa515
2022-01-24 18:40:00 +01:00
jenkins-bot 5fa81c4123 Merge "Restore scrolling to comments when clicking a bundled notification" 2022-01-22 17:24:32 +00:00