Added in 76289cdf73,
should no longer be needed since we switch to Parsoid's
HTML parser in 3e6ab2c4d2.
Change-Id: Ic0b7ed8089b71f2338e604f68d547759e069f0b2
When 'DiscussionToolsEnableMobile' was false, we were falling back to
the desktop configuration, rather than disabling everything.
When 'DiscussionToolsEnableMobile' was true, we were enabling the
selected features on every page, instead of only discussion pages.
Follow-up to b4f10c5638.
Bug: T302388
Change-Id: Ib4a42d5acd9da528e931c74de7a870d4be513d69
Also special-case thumbnail wrappers generated by
MediaTransformOutput::linkWrap, for compatibility with
TimedMediaHandler.
Bug: T301427
Bug: T302296
Change-Id: I7f48d8b2261507c5a33526c54109f5187d062ed3
Previously, we required a signature at the end of the comment.
This was a pretty rough heuristic that did not correctly handle
many comments that we would consider entirely properly signed
in CommentParser (e.g. comments wrapped in formatting like
<small>…</small>, comments with a post-scriptum or in parentheses,
or comments generated by various templates).
Now we process the user input using the same code that adds reply
links, and only add a signature when we detect that there really
isn't a signature (including template-generated), or if the signature
is in the wrong place and would result in the reply link showing up
in the wrong place as well (not at the end of the comment).
Bug: T278442
Bug: T268558
Bug: T278355
Bug: T291421
Bug: T282983
Change-Id: I46b6110af328ebdf93b7dfc2bd941e04391a1599
Also, in ThreadItem::getSinglePageTransclusionTitle(), we don't need
this terribly complicated method.
Change-Id: If02c09aaa2f4dd66b2bc253a1edec4ea107564ee
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
It adds white-space between block tags and strips invisible tags.
It may be slightly slower (it takes HTML as input rather than DOM, so
we need to serialize the HTML first and then call it, rather than only
find and concatenate text nodes), but the difference is negligible,
and it seems better to use this method than to try to re-implement it.
Test runtime went from ~9.0s to ~9.5s locally, when testing using:
php tests/phpunit/phpunit.php \
extensions/DiscussionTools/tests/phpunit/ThreadItemTest.php \
--filter getText
Bug: T219138
Change-Id: I0cb89ebd2160e1ef499b78573c6688f493a4c42f
For two given revisions, this API tells us which comments have
been added and which have been removed.
Can be used to highlight new comments, or check if the page
has been updated since we first loaded it.
Bug: T281624
Bug: T300504
Change-Id: Ia4d95ffe3b7cf2317cd8e7c0f034e09f64777ef3
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
Replaced with the more readable ::disableClientCache() method, added
in 1.38. Minimum MW version for this extension is already at 1.38.
Depends-On: I7c89e20528a0d91173f0edcb997dcae631935ee5
Change-Id: Idf1cf2fac3311f50ed3cbc420f7772b5c71b1992
This is now deployed on all wikis, and going forward I don't think
we need to make this configurable.
Change-Id: I231976267ba6cdfeec622efaa15983a84c330649
Goal: To be able to re-use or test the transformations we previously
performed in addWikitextReply() / addHtmlReply(), without requiring
a Comment object or adding the result as a reply.
Change-Id: I040c4be9b6b9bddba661f30fd0566f8850673074
* 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
After switching from source mode to visual mode, there will be some
Parsoid-generated white space between block nodes, which remains when
the reply is posted in visual mode.
Follow-up to e064f43499.
Bug: T300439
Change-Id: Ia5d2c06f0a4125e9f148eddd3235f95138c9d37f
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
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
This code previously ensured that the fragment identifier linking
to a section was only included if all events had the same section.
It doesn't actually seem worth the effort, since we handle scrolling
to the highlighted comments client-side anyway.
And the links were not quite correct, because we didn't parse and
strip the section title as expected by built-in Echo events. Just
use Echo's code for this.
Depends-On: Idb3a87fd18330f90a8cdc1276994d54288e17b28
Change-Id: Icae0d3654dd02109337ff8737b16f55bbd514f43
The following values for configuration variables are supported:
$wgDiscussionToolsReplyIndentation = 'invisible'; (default)
$wgDiscussionToolsReplyIndentation = 'bullet';
Bug: T259864
Change-Id: Icefad79630adc6ed35687498614e6a03ede1451b
$user->saveSettings() happens in AuthManager after the LocalUserCreated
hook finishes running.
Bug: T199393
Change-Id: Ic661dbe1ffaa3a5438373a33c10ad3053662d932
Reimplement getFullyCoveredSiblings() using compareRanges(), which
checks basically the same thing, but works better and I like it more.
Bug: T297034
Change-Id: I33dc1d088bdee984064315290e378bfbfa830b10
Previously: 569db3603c (2020-06).
Unfortunately we've found cases where the previous implementation
doesn't work correctly, resulting in comments being added to the wrong
pages or page corruption.
Bug: T289873
Bug: T298051
Change-Id: Id867b3005ebc46906d6df852a525fcaec9e6b19b
Comment boundaries are stored as a DOM parent node plus a child index.
Because of that, inserting anything into the DOM before a comment –
such as another comment's start/end markers – would cause us to insert
subsequent comments' markers into the wrong places.
This issue didn't affect many pages, because usually any parent node
would have just one comment in it. Only pages with comment boundaries
outside of any wrappers (directly inside the root node) were broken.
Just process the list in reverse to fix this.
Bug: T298096
Change-Id: Iccffc36b71e9fcf3d72c4db2b9459d39042f7a2d
We realized that any change made to Special:Preferences will set the
beta preference even if they didn't visit the beta tab, so we can't
actually tell if manual intent was involved. As such, we'll enroll
people regardless of their beta setting -- they can disable the feature
through regular preferences if they want, and that'll be respected.
Bug: T291307
Change-Id: I8c1cbf51060012e8e68af252da84944dfcc681d8
For logged out users we store their test state and an anonymous
identifier in local storage. So long as the test is enabled, we include
these in any logging that occurs.
This is done entirely client-side, to avoid any cache issues caused by
state depending on cookies from PHP for logged out users.
Bug: T291307
Change-Id: Ib39e2f2146cdfdac9df5690ee3de75718f0f2731
The global function wfWikiID() is deprecated since 1.35 and it's usages
should be replaced with WikiMap::getCurrentWikiId().
Bug: T298059
Change-Id: Ie878a5479b7427e9ffab7d7f92ee2802997e3161
Better describes that we are checking the editor used to make
the edit, rather than descibing some virtual "location".
Change options to 'discussiontoolsapi' and 'any'.
Change-Id: I3024517e70ed61c738e4bf46a2ac7b58c975d98a
Trigger off the (absence of the) new preference for tracking topic tool
usage.
Change the name of the bucket preference so anyone who was enrolled in
the prior abtest won't find themselves re-enrolled.
Update the abtest enrollment code so it explicitly sets the preference
for the feature. This is a trade-off -- it does mean that we'll need to
special-case *unenrollment* once the abtest is disabled if we want to
just quietly revert people to the wiki's default, but it also means that
Special:Preferences will be accurate.
Bug: T291307
Change-Id: I659679e05b65fc7db05e249114e5a7de4cf55816
According to Daniel it only worked by accident, and stopped working
after de63ad823abe:
getPageByReference() used to do an opportunistic lookup by ID when given
an instance of PageIdentity -- which is correct for EventDispatcher,
but problematic in the general case, causing T296063.
The correct thing to do here is to use getPageById(), since the canonical
association between revision and page is by page ID.
Bug: T297431
Change-Id: Icc1df0c9ca5345e65ef5f8daf0815013d7db0943
The 'legacyPrimary' links will take you to the section
the comment is in and should be used when you don't have
access to comment IDs.
Bug: T296018
Change-Id: I944feb90e7c3a69f81366f42fa110c58cac26dbb
This reverts commit 99b757465a.
Reason for revert: We may never need the 'behind-overlay' setting
and it is untested and probably broken.
Bug: T295816
Change-Id: I9e128862271697ece5241d0e98727174b42f54ff
* Add a N/A value for edit counts from anonymous users
* Only oversample with $wgDTSchemaEditAttemptStepOversample if the edit
is from DiscussionTools
* Consider $wgWMESchemaEditAttemptStepOversample for oversampling
Bug: T286076
Bug: T295995
Depends-On: Ieb3f6c6e1775c1ef53747c37003b17e3634d1c44
Change-Id: I91245a61dfbde8b5ec9b2893b9170cc4d73f7b0a
When our interface initialized on a page that the current user
recently edited (using the reply tool, the full-page source editor,
or any other way), check if any new automatic topic subscriptions
were added and update the interface to reflect that.
This requires doing some API requests after the page is loaded,
because adding auto-subscriptions happens asynchronously in a
DeferredUpdate (potentially after the user is already viewing
the page with their comment saved), and depends on the contents
of the edit.
(When using the reply tool, we could avoid this API request and
replicate the logic, but that's not implemented in this commit
to keep it simple.)
Bug: T284836
Change-Id: Ic0fabda0de4ebbc5e424f49641e6b03ebb4b7e6a
Usually this isn't a problem, because the comments are marked as
template-generated and we don't allow replying to them. But we had a
special case where we were trying to skip over some invisible
elements, which was causing us to skip into the middle of the
about-group in some cases. When Parsoid sees that, it serializes the
contents twice.
Bug: T290940
Change-Id: I9fe0b8d43ab874ccef371990799f77bfc46bc954
We do something similar in CommentItem.js with a moment object.
The object can be converted to a string when required.
Change-Id: Id7221e9201db0d89c3b771574634c878c9515ca0
* Use featuresEnabled.newtopictool to decide when new topic
links might exist on the page, instead of relying on
`#ca-addsection` existing. Change the logic of that feature
flag to check if __NONEWSECTIONLINK__ is on the page.
* Render the add-topic button locally in a hook to replace
the one suppressed by onMinervaNeueTalkPageOverlay. Do so
whenever the newtopictool feature is enabled (see above).
Bug: T270537
Change-Id: I3e3f7403b3b86bb84fcb75a8833919512519b70f
This should avoid them showing up in unexpected contexts where parser
output is used (e.g. API T292345, search T294168, action=render).
Also rename some variables to be directionality-neutral.
Bug: T292345
Bug: T294168
Change-Id: Ibcac44ee10f0842e205d9dd9a7f3a935ce0c690b
* Explode boundary point tuples passed to computePosition
* Note that we won't use previousSibling instead of array_reverse
* Simplify logic using xor
Change-Id: I927256e31b5e441aade91b4fd0d83d8f0d89afbe
This doesn't sort by the topic name, but the hidden
sub_id field, leading to a confusing order.
Bug: T273342
Change-Id: I6146abf05544d40c9ef0d2e8c58d020e5a5fa8a2
We always do our processing in the parser now, so we don't need the
marker comment to detect whether we've already processed the page.
Bonus: include the time taken by our processing in the limit report.
Bug: T291831
Change-Id: Ife7ddffbad1b1495b004739212002a98fdebe6c0
