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
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
Also special-case thumbnail wrappers generated by
MediaTransformOutput::linkWrap, for compatibility with
TimedMediaHandler.
Bug: T301427
Bug: T302296
Change-Id: I7f48d8b2261507c5a33526c54109f5187d062ed3
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
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
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
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
(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
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
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
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
* 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
