The "Reply" buttons were active when viewing an old revision of the
page (&oldid=1234). This was probably unintentional, and it would undo
all more recent comments if you saved yours.
However, I think it would be a useful feature. You often end up
viewing old revisions when reviewing changes to pages from your
watchlist or email notifications.
Now, when the reply widget is launched from an old revision, it will
try to find the relevant parent comment in the latest revision of the
page, and edit that revision when inserting the reply. If the parent
comment is gone, it shows a useful error message.
Bug: T235761
Change-Id: I8c5b631d3bfb62196fd219cbcd7d497408d187a7
The most common case of edit conflicts on talk pages is several people
responding to the same comment at the same time.[citation needed]
We can easily resolve this case by fetching the latest revision of the
page and re-running our code to insert a reply on it.
When we can't insert a reply, that probably means the parent comment
was deleted or moved, so display an error message indicating that
instead of the generic one.
Bug: T240643
Change-Id: Ic686acc747580d46779960211a02e9830a6ae86f
Previously, we only built the Parsoid document once (on page load) and
kept it around forever. Every time we tried to post a reply, it was
added to this document, even if it wasn't saved due to some error.
This resulted in duplicate replies when the user managed to actually
save.
Now we only keep around the HTML string and some metadata fetched from
the API, and rebuild the actual document every time before adding a
reply.
Bug: T245333
Change-Id: Ib1c344a7d613cdf67644aa243147c5e699c2c1e7
This ensures that expired tokens are refreshed and retried, while
invalid tokens caused by the user logging in/out cause an error. We
should think about displaying a better interface for the latter case.
Bug: T245327
Depends-On: I485f99e1f5f493262b0c9af22370da01adf1e09c
Change-Id: Ibc097ed68e3ae72223b0680ee8895f7884399958
Ignore the horizontal position of the comments' bounding boxes entirely.
It can be crazy because of de-indentation in the middle of the comment,
and even just text formatting with padding/margins (e.g. `<code>`) can
make it look weird. Just draw the rulers based on detected indentation.
Change-Id: Id4e5edf076d44bdedfb45958260d797daea29ed1
Native Range objects are automatically updated when the DOM elements
they refer to are affected (e.g. detached from the DOM, or their offset
changes because of siblings being added/removed).
This seemed harmless or maybe even slightly useful, but it turns out
it conflicts with VisualEditor, which has to wrap the entire page in a
new DOM node when it opens (and unwrap it when it closes), effectively
temporarily detaching it from the DOM, which destroys all our ranges.
Just use a plain object that stores the same data as a Range. And when
we need to use Range's API, we can simply construct a temporary one.
Bug: T241861
Change-Id: Iee64aa3d667877265ef8a59293c202e6478d7fb6
This sets up the tags:
* discussiontools
* discussiontools-reply
* discussiontools-edit (not yet implemented)
* discussiontools-newsection (not yet implemented)
The tags are flagged as user-addable, because otherwise they can't be
passed through to the VE API (at least, not without editing it so that
it explicitly knows about them, which seems like a strange
interdependency). It's assumed that letting users who know about the
tags add them to random changes via action=editchangetags would be
(a) the pettiest and most inconsequential vandalism possible, and
(b) unlikely to happen.
This relies upon I2c1d0f8d69bc03e5c1877c790247e165f160e966 in
VisualEditor to not also tag the edits with `visualeditor`.
Bug: T242184
Change-Id: I4e5e26afdd52279df242e1912f073b415b812c3b
The loop in parser.js assumed that there was always a heading before
any comments (not counting the page title, only section headings).
Bug: T243869
Change-Id: I3a0bb06716e75d4a17e25c40748673a071ee5f30
I don't like that I had to special-case `<p>` tags (top-level
comments) in this code. I feel like it should be possible to handle
top-level comments and replies in a generic way, but I couldn't find
a way to do it that actually worked.
Notes about changes to the behavior, based on the test cases:
* Given a top-level comment A, if there was a "list gap" in the
replies to it: previously new replies would be incorrectly added at
the location of the gap; now they are added after the last reply.
(T242822)
Example: "pl", comment at "08:23, 29 wrz 2018 (CEST)"
* Given a top-level comment A and a reply to it B that skips an
indentation level: previously new replies to A would be added with
the same indentation level as B; now they are added with the
indentation level of A plus one. (The old behavior wasn't a bug, and
this is an accidental effect of other changes, but it seems okay.)
Example: "pl", comment at "03:22, 30 wrz 2018 (CEST)"
and reply at "09:43, 30 wrz 2018 (CEST)"
* Given a top-level comment A, a reply to it B, and a following
top-level comment C that starts at the same indentation level as B:
previously new replies to A would be incorrectly added in the middle
of the comment C, due to the DOM list structure; now they are added
before C. (T241391)
(It seems that comment C was supposed to be a multi-line reply that
was wrongly indented. Unfortunately we have no way to distinguish
this case from a top-level multi-line comment that just happens to
start with a bullet list.)
Example: "pl", comments at "03:36, 24 paź 2018 (CEST)",
"08:35, 24 paź 2018 (CEST)", "17:14, 24 paź 2018 (CEST)"
* In the "en" example, there are some other changes where funnily
nested tags result in slightly different results with the new code.
They don't look important.
* In rare cases, we must split an existing list to add a reply in the
right place. (Basically add `</ul>` before the reply and `<ul>`
after, but it's a bit awkward in DOM terms.)
Example: split-list.html, comment "aaa"; also split-list2.html
(which is the result of saving the previous reply), comment "aaa"
* The modifier can no longer generate DOM that is invalid HTML, fixing
a FIXME in modifier.test.js (or at least, it doesn't happen in these
test cases any more).
Bug: T241391
Bug: T242822
Change-Id: I2a70db01e9a8916c5636bc59ea8490166966d5ec
Even when you have multiple signatures by multiple users in one
paragraph (or list item), it's still basically a single comment.
We don't want to offer multiple buttons to reply to it.
The changed parser test cases are illustrative:
* All affected comments in the "pl" example are comments with a
"post-scriptum", which is now more intuitively treated as part of
the main comment.
* The first comment in the "en" example would probably have been
better if it wasn't merged, but a weird use of the outdent template
causes us to not be able to distinguish that the two parts of the
comment display on separate lines.
* The last comment in the "en" example (isn't that neat?) was previously
incorrectly treated as two comments, because there's a timestamp in
the middle of it (the user is referring to another comment).
* Remaining affected comments in the "en" example are also comments
with a "post-scriptum" and their treatment is clearly better now.
It also accidentally fixes some problems with modifier tests (but not
all), where previously <dl> nodes would be inserted in the middle of
<p> nodes, to reply to the comments which are now merged.
Bug: T240640
Change-Id: I0f2d9238aff75d78286250affd323cd145661a11
Sepatate #teardown and #tryTeardown methods to make it
obvious what they do. Have <escape> call #tryTeardown
like the cancel button.
Change-Id: Ica0f3295bfee378bcd15d0b6a3ccea3c7917ad9b
* Add config option $wgDiscussionToolsUseVisualEditor (default false).
* Add new modules ext.discussionTools.ReplyWidgetPlain and ...ReplyWidgetVisual,
replacing ...ReplyWidget. Load only one of them depending on the config.
TODO:
* Also add the visual mode of VisualEditor, this only uses NWE now.
There is already code to support saving from it, but no mode
switcher tool
Co-Authored-By: Ed Sanders <esanders@wikimedia.org>
Co-Authored-By: Bartosz Dziewoński <matma.rex@gmail.com>
Change-Id: I9b6db865d51baf400fb715dc7aa68ccd8cdd4905
I did this wrong in a6147ffac8, the
'dt-init-done' class was never cleared. Put it on another element.
Bug: T241861
Change-Id: I136bd9c12bcc80cff01f5d26a8a53524f0c533c6
Unfortunately mw.Api#parse doesn't provide us with that part of the
response, so we have to manually construct the parameters.
Bug: T241193
Change-Id: Ie91d5ebc2ef483a69524b838dd3cb852e7c85cd2
The hook 'wikipage.content' fires whenever new "page content" is added
dynamically (e.g. after a VisualEditor edit). It's used by the code
for sortable tables, collapsible content, etc., to ensure they behave
correctly after the page content is changed without reloading the page.
We use this hook to add our "Reply" links. However, VisualEditor (and
NWE) also fires this hook for the contents of the edit notices box (to
support collapsible boxes there: T179315). Our code was crashing
because it could not find talk page content inside of that, and this
crashed VisualEditor as well.
Use .each() to handle any number of results (0, 1 or more), instead of
assuming there is always 1.
Bug: T241396
Change-Id: I877b1ae06bf1d7cd585ec6f9c1fb596cc3b86e7e
And actually discard the contents when they confirm.
For now this uses the generic editor message, but that can
be tweaked later.
Bug: T240271
Change-Id: I2dfa19b2cc7ac49d7efea37ac8c9429c75934a91
* Query parameters for the API must be in lowercase.
* Also, 'starttimestamp' was misspelled.
Bug: T240643
Change-Id: I6497770dfc3a9512af063b846c3f73aa5603b637
Method can be used by preview logic later. Whitespace trimming
avoids sig ending up on newline in a <pre>.
Change-Id: If6f06f17395af0c6645082c1b9493be87422c059
The packageFiles system makes it easier to export site config data
from PHP to JS, which we need a lot of, but it's awkward when mixing
it with defining and accessing classes via a namespace like mw.dt.
The only thing remaining in mw.dt is mw.dt.pageThreads, which is
described to be "for debugging", so we should keep it easy to type.
Also we still use the namespace for documenting classes.
Everything else can be reached by require()'ing a ResourceLoader
module, for example instead of `mw.dt.ui.ReplyWidget`
you'd do `require( 'ext.discussionTools.ReplyWidget' )`.
(When debugging from browser console, use `mw.loader.require` instead.)
Change-Id: I6496abcf58c21658d6fd0f3fc1db1f7380a89df7
Possible use cases:
* Matching comments between PHP and Parsoid HTML [implemented here]
* Finding the same comment in a different revision of a page
(e.g. while resolving an edit conflict, or to allow resuming
composition of autosaved comments) [implemented for highlighting
user's own posted comment only]
* Permanent links to comments [future]
The reasoning for this form of ID is:
* _Timestamp_ by itself is a nearly unique identifier, so it's a good
thing to start with
* Users may post multiple comments in one edit (or in many edits in
one minute), so we need the _sequential number_ to distinguish them
* _Username_ is probably not required, but it may reduce the need
for sequential numbers, and will help with human-readability if we
add permanent links
The ID remains stable when a new comment is added anywhere by anyone
(excepts comments within the same minute by the same user), or when a
section is renamed.
It's not always stable when a comment is moved or when an entire
section is moved or deleted (archived), but you can't have everything.
Change-Id: Idaae6427d659d12b82e37f1791bd03833632c7c0
