Skipping them could result in incorrect handling when RESTBase HTML is
outdated.
When a result for a given comment is not found, display an error
instead of assuming it is not transcluded.
Bug: T262065
Change-Id: I14a7a0a25d5181b5c49bd5677f0c002dce5a3cb9
We depended on the oldid (wgCurRevisionId) changing after a reply is
posted, but it will not change if we posted to a transcluded page.
Bug: T266275
Change-Id: I1baa1f2227134b73fd663de2fee3ea96a2f9b183
We avoided fixing these because it causes changes in just about all of
the test data, which is annoying when reviewing or blaming changes.
But the previous several commits also caused changes in just about all
of the test data, so we might as well do this too.
Change-Id: I83b64d83b6f12c04dc06c0cadff7cdd89417e137
To avoid old threads re-appearing on popular pages when someone
uses a vague title (e.g. dozens of threads titled "question" on
[[Wikipedia:Help desk]]: https://w.wiki/fbN), include the oldest
timestamp in the thread (i.e. date the thread was started) in the
heading ID.
Bug: T264478
Change-Id: If918bfd5e025248923d1939bc86916697ead95a0
Sequential numbers aren't great because they change when an earlier
comment is archived. Parent comment/heading IDs should change less
often.
This also makes much more sense for disambiguating subsections,
e.g. a dozen identical ===Votes=== sections for a dozen proposals.
Bug: T264478
Change-Id: I466454984fd919ebef35f2b37ddb5d86dc842996
Our threads now also contain all replies to their sub-threads.
This is similar to how sections work in MediaWiki, where the parent
section also contains the content of all the lower-level sections.
We're going to need this for notifications about replies in a thread.
Bug: T264478
Change-Id: I241fc58e2088a7555942824b0f184ed21e3a8b6f
Previously, only comments could have IDs, because we only needed IDs
for replying. But we might also use them for notifications soon.
Bug: T264478
Change-Id: I1bcad02bf17ab54bc5028a959543c10f0430836b
The output of CommentFormatter::addReplyLinks() and consequently
ThreadItem::jsonSerialize() can end up in the HTTP cache (Varnish) on
Wikimedia wikis. We need to consider that when changing that code.
Introduce a concept of legacy ID (generated by the older algorithm
after it changes), add some placeholder code that will generate them
in the future, and update some code to find comments by either normal
or legacy IDs.
Add dire comments in a bunch of places (as if that ever helps).
Bug: T264478
Change-Id: I4368f366800ab21b8b184b09378037614fdecd33
"This modifies the original objects…" – I feel like this is obvious
now, but maybe it wasn't so obvious when this code was structured
differently before a2431fe006. Also,
it refers to a variable that doesn't exist.
"FIXME this will clone the reply…" – No, actually, it will not.
It would if replies were associative arrays, but they are objects,
and have always been, ever since the PHP parser was merged in
7b7a2cd69c. Maybe they were arrays
once in Roan's mind before he pushed that for review.
Change-Id: I1348e111699fdbde99cd1f9ef45d8f465f7391b0
* Add the preference
* Only display it when the reply tool is enabled
* Use it when opening the reply tool
* Save it when the menu is toggled from the reply tool interface
Bug: T261539
Change-Id: Icb8fa6b3f1e9a3644669f21b08f34ea8c175f2f9
Always select the default reply if it looks unchanged, i.e.
we see '...*/ Reply' at the end of the summary.
Bug: T263062
Change-Id: I0a79d9e5072d9d9df16c93435502f67524e2d2bc
Through trial and error, I found that adding `display: inline-flex;`
avoids the issue, and does not seem to have any negative effects in
Chromium or other browsers.
Bug: T260072
Change-Id: I9a9ca1fdb57bb7dd6c1a0a70e330a2a503c8ec8e
When a timestamp directly followed a `<div>…</div>` tag (or perhaps
some other wrapper containing lots of content), we would detect the
username from the earliest links in the wrapper (furthest from the
timestamp), rather than the latest links (closest to the timestamp).
Bug: T262573
Change-Id: Id16449a86a731b13dc79846bb30ecf6554e26f1d
The wikitext parser outputs `<p><br></p>` for empty paragraphs, so we
need to ignore `<br>` tags when searching for an "interesting" node
that marks the beginning of a comment. Otherwise the empty paragraphs
mess up the detection of indentation levels.
Bug: T264116
Change-Id: I84a97ab577baa7336b78935ccdc48041ecfc231a
The same ReplyWidget instance can be re-used when the user cancels
replying and then starts replying again to the same comment.
Previously, the initial values passed to the constructor (when switching
modes) would still persist in this case, even if they were incorrect.
Bug: T263061
Change-Id: I3dff007456bfd4f3149c718ae72fbf373a2e2913
* Export parser data (date format, digits, timezone names, and
messages for weekday/month names) converted to language variants
* Update the parsers to try matching using every variant, in case
the page is displayed in non-default variant (and to avoid
problems with incomplete variant conversion)
Bug: T259818
Change-Id: I04d73992cd31ce06fa79f87df0c0a53d7efc3c58
Now consistently uses setReadOnly instead of setDisabled.
Slight styling regression was fixed in I01e11c2ed.
Change-Id: Ice7ce00929ff9a53d0fb533f1094e7324749f3a4
PHP was counting UTF-8 bytes, JS was counting UTF-16 bytes.
Both should have been counting codepoints (although it doesn't
really matter as long as they both count the same things).
I noticed the issue after adding some tests using the Cyrillic
script, when one case had different results in PHP and JS:
Id25b537fecd789640c209ff7f30e777455a3aece.
Change-Id: Ic31240678f71ba48e6ec202126bf490cea12bb66
Move the code so that we check for "?title=" query parameter first,
because we don't handle this right in the other code path.
Use parse_url() instead of wfParseUrl() because the latter doesn't
accept relative URLs, and we don't care about the other differences.
Bug: T261711
Depends-On: I4da952876e1c3d1a41d06b51f7e26015ff5e34d7
Change-Id: I70fac2b41befd782b0a47a4f726ae748dc0f775d
The PHP code incorrectly assumed that the digits are single-byte in
UTF-8, which is never the case (except for 0-9).
The JS code worked correctly because it uses UTF-16 strings, so the
bug would only affect non-BMP digits there. This was noted in a TODO
comment, but we overlooked it when reimplementing in PHP.
Instead of a string of 10 characters, use an array of 10
single-character strings.
Bug: T261706
Change-Id: Ic5421382474c88f003424799c53ff473d99cce92
Without this, the reply widget would be inserted before the reply button,
in the middle of the paragraph.
Follow-up to e36dc8e78a.
Change-Id: I5e22a0a0b70e8da395eb23dd6ae80af70840a2e2
This got lost when the watchlist updating code was refactored
out into controller.js, as it assumes that 'data' (the post data)
is still available.
Bug: T261362
Change-Id: Id274e7b8518b62dddbc3d4095d9f6cab17aa17cd
When a comment ended before the end of a paragraph, the next
comment would begin right there in the middle of the paragraph.
This could result in the detected indentation level of that
comment being incorrect, and replies being inserted in wrong
places, as seen in the 'signatures-funny' test case.
The code moved to the parser was previously repeated twice in
addListItem() and addReplyLink(), which should have been a hint
that something isn't quite right.
Also, fix the code guarding against overlapping signatures,
now that signatures may not be at the end of a comment.
Bug: T260855
Change-Id: Ic26a87642f8a15d5de2f7073d4d8176b299c7f94
We were accidentally resolving the promise with `undefined`, rather
than rejecting it. This later caused "TypeError: Cannot read property
'linterrors' of undefined" in controller#checkCommentOnPage.
Bug: T260294
Change-Id: I29418988023d86140cfa110c0c348059b293a716
Causes page corruption, in a new way we haven't seen before.
* Revert "Move page updating logic to controller.js"
This reverts commit 54fdc6de06.
* Revert "ReplyWidget: Move clear methods from #teardown to #clear"
This reverts commit 9b811a94e0.
* Revert "ApiDiscussionToolsEdit: Do not pass 'basetimestamp'"
This reverts commit 7de5938a6f.
* Revert "Use DOMCompat::getOuterHTML instead of doc->saveHTML()"
This reverts commit 7b2448d2f0.
* Revert "CommentController: Remove remains of client-side edit conflict handling"
This reverts commit 2d038af705.
* Revert "Restore error message for when comment is deleted while replying"
This reverts commit 655c0526d6.
* Revert "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
This reverts commit 9d0fc184fe.
* Revert "Create a 'transcludedfrom' API endpoint"
This reverts commit 5d8f3b9051.
* Revert "Edit API for replies"
This reverts commit 8829a1a412.
Bug: T259855
Change-Id: I6419408c6194ec0afa6b8ee604b12c1a24c6ac7b
Previously, parser would output offsets that don't exist in their
containers, because we were pretending that entities are parts of
their neighboring text nodes.
Turns out it's much easier to do it right when going backwards.
Change-Id: I9bccca2d403f1a976ae517449989170cdd99721e
In commit 8829a1a412 we kept this code
when adding the reply API, but on second thought it doesn't actually
make sense.
The reply API should never fail with an edit conflict normally, since
it makes the edit on the latest version of the page. If it does fail,
I think it's better to just show the error message. Current behavior
is to retry, and if that doesn't actually solve the problem, this ends
up in an infinite loop.
Bug: T252558
Change-Id: I5e0dcb2c42e5de4f18e99b8a761ca048a430b31e
The footer was already laid out using flexbox and just required some
CSS tweaks to enable wrapping.
The header required some DOM changes, now also uses flexbox, and wraps
in a similar manner.
Bug: T259320
Change-Id: I6f6a86932a108037bf1d70f077c372654318be26
This reverts commit 96953647c3.
* Re-apply "Edit API for replies"
This applies commit 8829a1a412.
* Re-apply "Create a 'transcludedfrom' API endpoint"
This applies commit 5d8f3b9051.
* Re-apply "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
This applies commit 9d0fc184fe.
* Re-apply "Restore error message for when comment is deleted while replying"
This applies commit 655c0526d6.
Change-Id: Id20d21899f87464636022aa0683f8c03e0060117
Causes page corruption.
* Revert "Restore error message for when comment is deleted while replying"
This reverts commit 655c0526d6.
* Revert "Use transcluded from API to avoid ever fetching Parsoid DOM in client"
This reverts commit 9d0fc184fe.
* Revert "Create a 'transcludedfrom' API endpoint"
This reverts commit 5d8f3b9051.
* Revert "Edit API for replies"
This reverts commit 8829a1a412.
Bug: T259855
Change-Id: I98036f14dd900b51f20e98696e31b9b618eceee1
When adding a reply, we take a node at the end of the previous comment,
compare that comment's indentation level to the expected indentation level
of the reply, and add (or remove) that number of wrapper lists.
The existing code did not consider that comments may have lists within
them, and so the indentation of that node may not match the indentation
of the comment.
Bug: T252702
Change-Id: Icc5ff19783d2b213bff99f283cb0599a8b5c1ab4
Previously we preferred that, but used '*' (<ul><li>) when the parent
comment or the previous reply also used it.
Bug: T252708
Change-Id: I3abf606da6693905764f1be745fad999fdf57fbe
* Remove the existing approach for detecting signatures that only
worked in source mode; remove autoSignWikitext()
* Use the same approach for auto-signing in source mode as we have
already used in visual
* In both modes, detect whether the user has already typed a signature
at the end of their comment in the modifier, and if so, don't add a
signature
* Add test cases for the detection
Bug: T255738
Change-Id: I791d3035cb1ffc33ce3966d4617a25d08700c35b
* Pass rootNode to the constructor
* Rename getters to match CommentItem/HeadingItem/ThreadItem
value classes.
* Always build the thread tree so CommentItem's always have
and ID and replies/parent.
Change-Id: I508be9534de59016ff806e3d84edcbb1c76cb0c6
Load 'ext.visualEditor.mwsignature' (which implements VE's existing
handling for signatures), then subclass and override a bunch of things
in order to:
* Replace the context menu with a note that you don't need to type the
signature when commenting using the reply widget
* Override the sequence/command to insert signature so that it selects
it afterwards and thus displays the context menu
* Treat signatures as signature nodes when switching from wikitext,
instead of the normal pre-save transform turning them into regular
links and text
Bug: T255738
Change-Id: Icb542451c2307ab51e56bd627804096c7b5552c8
Instead of doing a separate tree walk and finding all timestamps
separately, make it part of the getComments tree walk, and find
timestamps one at a time.
Change-Id: I47f466eaf228504faa189fd99e07493bc7f022cd
This is a regression from when we added a wikitext parameter to
preparePreview. Calling getValue in visual mode converts the whole
DM to HTML, which is fairly expensive, especially as there are
no previews in visual mode.
Change-Id: Ieed61b0d7741e7204a942a7181923da0502981cd
We can update the local filtered list before then, but then we would
have to do two updates for each keystroke, one to do an instant local
filter and another to back-fill the list when the remote results
become available. Currently CompletionAction is not set up for this,
and this might be confusing behaviour.
Bug: T256974
Change-Id: I6194cdcd6459be17fb142e644d73c9ec4036ba08
Separate out conversion to DOM nodes, and update
now that postReply doesn't need to return a promise.
Change-Id: Iab6192ba252244009d2374c9423d14279e8d1c4f
This is helpful when you ping the wrong person and need to correct your input.
Logically depends on I4c34e9368692b0ee4e7ca0f18ba2940406c62a9a
Change-Id: Iea89bdb5d93fe64902b692f04dd3a2e84e5517c3
* Run the 'wikipage.content' hook before our initialization.
This way collapsible elements and other changes to the page layout
are processed before we measure where the new reply is and try to
highlight and scroll it into view. (T252903)
* Remove the code dealing with 'mw-parser-output' and 'dt-init-done'.
This was needed to avoid initializing twice on the same element,
which can't happen now. (T254807)
This is much closer to the original approach Ed proposed in
I05a3c766668999f05cfe06473652429025595196 before I changed it:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/550693/7..8
Bug: T252903
Bug: T254807
Change-Id: Ibc3fcbd3c92c8eceda19b68cee9e69f6e92456f5
The automatic event from core didn't catch this, because it's not an action
method called "open".
Bug: T255638
Change-Id: Ifa456e850a8edb374df098e21b46bb872416ae55
Parsoid doesn't understand syntax expanded during pre-save transform,
e.g. [[User:Foo|]] or ~~~~, and it gets nowikied when switching back.
Change-Id: I791426d9d6c0a1399b1e95039b11e43a3e5bf79f
Current code breaks in other languages which don't use whitespace as a
word separator. The word-separator message is (for reasons) not reliably
set to something other than " " on wikis for those languages, so we
can't use it as a trigger for just not caring.
Bug: T255153
Change-Id: I1ff929811af59fd7a7bf7e8dfc1cd121a2c4db4e
This was removed upstream due to a regression.
We don't need it right now, but may need it in the future to
support editing, so replace with a TODO.
Change-Id: Ia73657080241319df309f3b6566ac0ac36a5909d
