Notifications are bundled by section, so instead of linking to the
comment, link to the section.
Additionally, add a parameter to the URL listing all the comment IDs
from the bundle, and highlight them all and scroll to the topmost one.
Having to handle both URL fragments and URL query parameters makes
this code kind of a mess :(
Also, some unexpected changes became necessary:
* EventDispatcher.php: Store the section title in events using
HeadingItem::getLinkableTitle() instead of ThreadItem::getText().
The result is mostly the same, except in case of wacky markup like
images or extension tags. We can more reliably use it to link to the
section on the page, and we already use getLinkableTitle() when
generating edit summaries in the reply tool for this reason.
* dt.init.less: Change the mix-blend-mode for the highlights from
'multiply' to 'darken', so that multiple overlapping highlights do
not look more opaque. This affects how the highlights look on
non-white backgrounds and images (they're less blue, and on darker
backgrounds entirely invisible), but it seems worth it.
Bug: T286620
Change-Id: I21bb5c003abc2747f0350d3f3af558dfb55693e9
This was copied from somewhere else (probably VisualEditor), but it is
not needed here.
Using the multipart/form-data encoding is beneficial when sending long
binary data (such as the compressed HTML VisualEditor sends), and is
also required when uploading files, but it is not helpful when sending
a short query like we do here.
Also rename a variable.
Change-Id: I9bcce2ce1ca7c218e4cd147960d1070dd23ea9fa
This will avoid a flash of the empty-state while we're reloading the
page to get new tabs.
Refactor out the new topic controller's clear behavior from its teardown
behavior, so we can still wipe out the storage when redirecting.
Bug: T288314
Bug: T288320
Change-Id: I6a5313b5e5b3bc9925e5cdaea04d8fbd3dc796af
This includes the dtrepliedto URL functionality from
I3f81e4d77faed367606e47678b8896051982359d.
Bug: T274831
Bug: T274832
Bug: T277329
Change-Id: I035d04f30c8312b0cb42902d3bf940df1482ffb3
This is the same method as used by VisualEditor
(ve.init.mw.DesktopArticleTarget.prototype.replacePageContent).
Bug: T275698
Change-Id: Idcf7c79b8d5565b0ae36c6e9d42b66662c1acc8d
Problems with the current setup:
* Each CommentController must have exactly one link.
For T277371 we want multiple, and for T282205 we might want zero.
* CommentController objects must be constructed immediately.
They are implemented to make this pretty fast, but it's still
unnecessary work to do on page load.
* Only one link may be activated at a time, and activating one affects
the styling of others, so CommentController has to use global state
to check if it can set up and to update them.
Instead introduce ReplyLinksController, which knows about all reply
links and which one is active, and emits events that allow
CommentControllers to be constructed on demand.
Change-Id: Iabdeded2e71e598ae78703a6ff9410d0cfba397c
Considerations:
* Using the same edit notices as VisualEditor, except 'anoneditwarning'
* No extra frame/styling is added (on Wikimedia wikis, the notices
often already have them)
* 'talkpagetext' message is not shown (on Wikimedia wikis, they are
mostly about signing your posts with tildes, which is not necessary)
Bug: T269033
Change-Id: Idc5ff29f093c75a14c3a3479888295d5bf630f6d
Previously, our highlights were placed in a node at the end of
the page, and positioned absolutely in relation to the whole
page. Now we insert the highlight in the DOM near the comment,
and position it in relation to that.
This way it remains positioned correctly when the page shifts
(e.g. collapsing the table of contents), and disappears when
the page content is hidden (e.g. opening visual editor).
Bug: T281471
Change-Id: I60afc4b94b2e23376105638542563e595a1811d9
Per Manuel Arostegui in T263817#7033384. The limit is 5000.
(I picked it arbitrarily, there's no real rationale for it.)
Also log a warning when any user reaches half of the limit,
so that we might make a decision about changing this mechanism
before it starts affecting users. Maybe at that time we'll
have data to show that it's safe to remove the limit.
Bug: T263817
Change-Id: I18a8ee0ad7383759229c5721d5253fb591457d4d
As of 7ad6328223, we also use this data
to check whether comments exist on the page, not only whether they're
transcluded.
Follow-up to 42ce942c86.
Bug: T275821
Bug: T273413
Change-Id: I95eb85354e7b84cc10ab703d28315d0667696f4c
The information is already included in the VisualEditor metadata request.
Bug: T276393
Bug: T270803
Change-Id: I45a232dcd23418da0711834bcc369a9a718006b0
The existing comment IDs can't be used to find the same comment on
a different revision or page (when it's transcluded), because they
depend on the comment's parent and its position on the page.
Comment names depend only on the author and timestamp. The trade-off
is that they can't distinguish comments posted within the same minute,
or in the same edit, so we will still need the IDs sometimes.
Prefer using comment names when replying, if they're not ambiguous.
This fixes T273413 and T275821.
Heading names depend on the author and timestamp of the oldest comment.
This way we don't have to detect changes to the heading text, but we
can't distinguish headings without any comments.
Bug: T274685
Bug: T273413
Bug: T275821
Change-Id: Id85c50ba38d1e532cec106708c077b908a3fcd49
Longer, but follows the style guide and less likely to conflict.
We need to account for init classes in the cache being around for
a while.
Change-Id: I738bc93393850db320fdbda2b003ca8ac40556da
If the user is clicking on a new topic link, and a reply widget is
open, attempt to close it instead of doing nothing.
Bug: T272545
Change-Id: I1903f5ae4c9e98c4b3a4703ad0e44d772894592a
We'd like the [reply] links to behave differently if other
CommentControllers are already active, but each CommentController
doesn't know anything about others; only the main controller.js does.
Change-Id: Ic21b2d40d213a325509822f703709f52aa8dc8d7
If the module is loaded on a page where DiscussionTools is
not supposed to be enabled, wgDiscussionToolsFeaturesEnabled
will be undefined, and the code will crash before it can set
the cookie that enables it for future page views.
Bug: T272850
Change-Id: Ia1c40cfc3cbee62823f1806bd20229883905677a
Don't assume a feature is available because the code has
loaded and the user option is set. Export the logic from
Hooks.php to the client.
Change-Id: Ica0e58de7ed0d59e3b09645193eb2b691ae41c39
1. Extend the JS modifier to allow adding top-level comments
(that is, replies to headings). PHP modifier doesn't do this
because we'll save the changes using paction=addtopic instead.
2. Subclass CommentController to allow adding a new heading and a
top-level comment underneath it at the same time.
3. A lot of ugly code in ReplyWidget to customize the interface
for this case. Much of it should probably be moved to
CommentController/NewTopicController.
Bug: T267595
Change-Id: I9c707bb7f7aae1b92c72fb4dee436490f8c8409b
* Use a slightly different color
* Use 'mix-blend-mode: multiply' when available to avoid fading
the text color
* Add fade-in animation and make fade-out animation slower
Bug: T268994
Change-Id: I210ed4fd55c3dc184d13daf915fa93bee3699ad5
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
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
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
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
* 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
* 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
This means we have an instance for each comment, which makes keeping
track of local variables much simpler, and will make switching out
the widgets easier.
Change-Id: I7584870225b1a14146852987d6a40ad05957387c
When the user clicks a "Reply" link on a page that is affected by the
'fostered' lint error (indicating fostered content in the HTML
representation), display an error and refuse to edit it, as Parsoid's
transformations will damage the page content.
The error message includes a link to documentation about lint errors,
and a link to the editor that will highlight the error location.
Depends-On: I723ec766d1244d117f8d624440026fe5af0d3403
Bug: T246481
Change-Id: Ic60cb58f98d10dc9b113469e5d3bbfb2d2b0564f
Extra lines currently render empty list items which are invisible.
Remove these as the user probably didn't intend from them to appear
in the wikiext.
Bug: T249867
Change-Id: I75c1192a94b918783d362d38cd91a508bb169d56
3 or 5 tilde signatures will be assumed to be erroneous and fixed
to 4 tilde signatures. This will be visible in the preview so shouldn't
come as a suprise to users.
Bug: T245628
Change-Id: I741f0761a6fb10c99cf3239ac5c6c7e1a2b872c7
The section wrappers can be marked as template-affected when the
previous or next section is transcluded, causing comments to be
unnecessarily uneditable. The new test case demonstrates this.
Depends-On: I03bc455d5484a6c51f3fa2397c64936b829fe7e3
Change-Id: I895a04990d79a3475d778b4fef054ea0bb076f0b
* When we discover the comment comes from a transcluded page, follow
the transclusion to find the source page. We follow transclusions
recursively, up to an arbitrary limit of 10.
* In the reply widget, display the title of the page where we will
save the reply, to avoid users confused why their edit won't show up
in the history. In the wikitext workflow this is done by redirecting
the user to the edited page at the end, but it seems less surprising
to stay on the current page.
* After saving the reply, we must purge the current page, otherwise
the new content will not be immediately visible on it.
Bug: T247535
Change-Id: I1c6631aa65a2fce6c1c2f0dd4a8c7aa6389caf94
The rejection handling callback was accidentally resolving the
promise, which resulted in an error like 'TypeError: Cannot read
property '$element' of undefined' when the resolution value was
used.
Fixing this reveals that we weren't removing the placeholder list
element correctly. This wasn't immediately visible because of the
.empty() call in the next resolution callback, but it would have
caused something similar to T245574.
Follow-up to 6964f0c965.
Change-Id: I3aeb9a86046c4ccaa6c39301edc7285d02b0320c
Previously you'd only learn about the issue when saving failed.
Now a modal alert dialog with the error message appears.
This means that we have to wait for the loading to finish before we
can display the ReplyWidget now... this should not be noticeable,
since we preload in #init.
Bug: T247533
Change-Id: I5468e67c449d530a0d15f69bff954d37a5b6a14c
This has two benefits:
* Allows detecting a conflict if two edits are saved in the same second
* Doesn't ignore conflicts with yourself (T246726)
Depends-On: Id7565018f66860b5c2ba688777508db1b88700ae
Bug: T246726
Change-Id: I22eaa1af5692854870d31e08b171a070a2fda0de
This is a problem we can detect at the loading stage, rather than at
the saving stage, so move it from #postReply to #getParsoidCommentData.
Follow-up to e3e4ef9de4.
Change-Id: I19362399f9ff2fdc487ea4900654bc61d990575f
When trying to reply to a comment that is inside a transclusion,
detect if it's transcluded from a subpage or simply wrapped in a
template, and show appropriate error messages.
References:
* VisualEditor ve.dm.Converter#getAboutGroup()
* VisualEditor ve.dm.ModelRegistry#matchElement()
* Parsoid Linter#findEnclosingTemplateName()
Bug: T245694
Change-Id: If3dd1ebbf1d02ee4379c200019bfc3a8ec02325b
We disable the other reply links using 'pointer-events: none' in CSS,
but IE 11 doesn't support that.
I tried hiding them instead and using @supports rules in CSS to apply
'pointer-events: none', but the result was worse than this.
Change-Id: Iab2bfe9c623f3d32cce9776277f33483155a0c42
We always pass the same arguments to the controller method and
two of those are global config values, so just pass the comment ID
each time it is used instead.
Change-Id: Ic68c70bdadb29310e930dd10fd6c6137d01ad22f
Previously they were added at the end of the text node containing the
timestamp, which was usually the end of the line, but not always.
And also fix the same problem for inserting the actual replies (or
reply widgets). This replaces an undocumented hack that prevented our
own reply links from triggering this bug (without it, the reply widget
would be inserted before the reply link rather than after).
While we're here, remove unintentional spacing that appeared before
some reply links, caused by trailing whitespace in text nodes.
Add tests for all of the above.
Bug: T245695
Change-Id: I354b63e2446bb996176a2e3d76abf944127f307e
This is the same as in VisualEditor. ve.parseXhtml has workarounds for
IE 11 bugs that would cause dirty diffs when saving.
Note that we must use ve.serializeXhtml to convert the document back
to a HTML string, but this is already the case (the conversion happens
in mw.libs.ve.targetSaver.saveDoc).
Change-Id: Ib6dec0002eaf33fc0d4a45331a6d38e5c5d7ab8c
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
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
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
* 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
* 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
