Commit graph

247 commits

Author SHA1 Message Date
Bartosz Dziewoński 4e135c7f07 Use 'baserevid' instead of 'basetimestamp' for edit conflict detection
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
2020-03-12 16:31:32 +00:00
Bartosz Dziewoński 1fff57fdff controller: Move code for transcluded comment errors
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
2020-03-09 21:59:12 +01:00
Bartosz Dziewoński e3e4ef9de4 parser: Detect comments transcluded from another page
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
2020-03-09 20:28:56 +01:00
Ed Sanders 3af3f3ed8c Wrap reply link in container so it may contain more links in future
For example 'Edit'.

Change-Id: I3d3027724cfd69a6719932bb2cb80fa711010fc4
2020-03-09 12:52:18 +00:00
Ed Sanders 23966cc098 Move wikitext comment building to the controller
Change-Id: I6374ea570f093ae5286c14ed49a9be4f32f23ff5
2020-03-09 12:52:06 +00:00
jenkins-bot 9913a77d61 Merge "controller: apply ve.fixBase to the parsed Parsoid response" 2020-03-05 16:09:55 +00:00
jenkins-bot f7c3b701f9 Merge "Only allow opening one reply widget at once (on IE 11)" 2020-03-05 14:46:37 +00:00
Bartosz Dziewoński 898db0d57a Only allow opening one reply widget at once (on IE 11)
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
2020-03-05 13:31:29 +01:00
David Lynch 0085b7c912 controller: apply ve.fixBase to the parsed Parsoid response
Also, add the missing dependency on ext.visualEditor.core.utils.parsing.

Bug: T245781
Change-Id: I26130f1afd7dd93012aea8a24943d966250c2472
2020-03-04 15:46:08 -06:00
Ed Sanders 3274b0c9df Move edit conflict retry code to controller
Change-Id: Id14e93624d1828253402c04c97193fd686f67d9f
2020-03-03 13:25:14 +00:00
Ed Sanders 126266c741 Remove somewhat useless ReplyWidget.prototype.getParsoidCommentData
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
2020-03-03 13:20:23 +00:00
Bartosz Dziewoński 606d6b34ec Add reply links at the end of a line, even if the signature is in the middle
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
2020-03-02 21:39:37 +01:00
Bartosz Dziewoński 711d5c4371 Parse Parsoid document as XHTML to avoid IE 11 bugs
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
2020-03-02 18:14:35 +01:00
Bartosz Dziewoński d068d2ef2c Clean up the interface after discarding a reply
Bug: T245574
Change-Id: I016a7a5c44e0d15a153143177976cceb8d6d3d1b
2020-02-27 18:09:34 +01:00
David Lynch fb006e6373 Instrumentation
Bug: T243364
Change-Id: I8573993db0dad408f09202e548206b009c106cc9
2020-02-24 19:50:27 +01:00
jenkins-bot 37bf81bf37 Merge "When launched from an old revision, reply to latest revision" 2020-02-24 16:23:21 +00:00
jenkins-bot 4d6bdcafb1 Merge "Try to resolve edit conflicts" 2020-02-24 16:23:20 +00:00
jenkins-bot 1dfa501cc6 Merge "Rebuild Parsoid document before attempting to save" 2020-02-24 15:53:54 +00:00
jenkins-bot e94ebddaa4 Merge "Use built-in mw.Api 'badtoken' handling, also 'assert'/'assertuser'" 2020-02-24 15:48:01 +00:00
Bartosz Dziewoński da11a3be73 When launched from an old revision, reply to latest revision
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
2020-02-21 17:09:47 +00:00
Bartosz Dziewoński 7761f62b42 Fix edit summary for comments in 0th section (no heading)
Bug: T245765
Change-Id: I9eb4726ef096b8d7459cc1409814514ec1dc89ae
2020-02-21 00:44:42 +01:00
Bartosz Dziewoński 7ea6cdf326 Try to resolve edit conflicts
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
2020-02-15 05:43:14 +01:00
Bartosz Dziewoński 6f404e5ce2 Rebuild Parsoid document before attempting to save
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
2020-02-15 05:09:34 +01:00
Bartosz Dziewoński 80cddf549c Use built-in mw.Api 'badtoken' handling, also 'assert'/'assertuser'
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
2020-02-15 03:07:02 +01:00
David Lynch 16215bbea2 Change tags method so anon edits will get tagged
Bug: T242184
Change-Id: I38baddc0febe02f6d2321be616adc018c87b5a54
2020-02-07 01:19:11 -06:00
David Lynch 2fcc23ae61 Add editor-mode changetags
Bug: T242184
Change-Id: Ib307e44ec883b01e0705c889f97e16326519f4c2
2020-02-06 20:22:27 +01:00
Bartosz Dziewoński e5e6fdd3af Stop using native Range objects, they're too annoying
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
2020-02-05 19:42:03 +01:00
David Lynch 664b2890d7 Tag DiscussionTools edits
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
2020-02-04 11:15:18 -06:00
jenkins-bot f735e47fdd Merge "Attach highlights to positioned container" 2020-01-30 00:07:04 +00:00
jenkins-bot 3b4ae40658 Merge "Don't use RangeFix for node rects" 2020-01-30 00:02:49 +00:00
Ed Sanders 4b3d92a5e9 Attach highlights to positioned container
Bug: T240639
Change-Id: I41a547acd3e5d7e9a49f99fc5ef35738d038007e
2020-01-29 15:22:20 -08:00
Ed Sanders 087d1eb428 Don't use RangeFix for node rects
Change-Id: I60e45ee9288fbdad26bb85997a7b4a24234fa8b4
2020-01-29 15:22:19 -08:00
Ed Sanders 7de6b4e04a Use simple targetSaver API call
Depends-On: Ida47968c995166b1dca36fc9fe28fac374010564
Change-Id: Id3decabb803a65d79cb09668bde4e0b498ad057c
2020-01-24 15:45:43 -08:00
Bartosz Dziewoński 890588f36a Pick reply insertion point based on parser tree, not DOM tree
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
2020-01-23 21:13:12 +01:00
Ed Sanders 2f1cf65233 Option to integrate VisualEditor instead of textarea
* 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
2020-01-07 22:15:03 +00:00
jenkins-bot b1d5e2953a Merge "Add beforeunload handlers" 2019-12-13 19:00:40 +00:00
Ed Sanders 179b29a598 Add beforeunload handlers
Bug: T240259
Change-Id: I376ea116a0fd1fdf794d9eb9e1d15f3d34babd88
2019-12-13 18:11:22 +00:00
Bartosz Dziewoński 0568d42042 Correct typos to fix edit conflict detection
* Query parameters for the API must be in lowercase.
* Also, 'starttimestamp' was misspelled.

Bug: T240643
Change-Id: I6497770dfc3a9512af063b846c3f73aa5603b637
2019-12-13 15:29:42 +01:00
Ed Sanders 763450db24 Create autoSign method and add whitespace trimming.
Method can be used by preview logic later. Whitespace trimming
avoids sig ending up on newline in a <pre>.

Change-Id: If6f06f17395af0c6645082c1b9493be87422c059
2019-12-12 15:28:42 +00:00
Bartosz Dziewoński 3e5a67010f Use module.exports/require() rather than mw.dt namespace for defining classes
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
2019-12-10 22:47:40 +01:00
Bartosz Dziewoński da668b72d5 Identify comments by username+timestamp+seq
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
2019-12-09 13:45:31 +00:00
Ed Sanders 682732897d i18n all the things
Change-Id: I01da9a88cd69facfeb33b37a727d1cd65c12a78d
2019-12-06 18:51:02 +00:00
jenkins-bot a7db61d43f Merge "Highlight comment after saving" 2019-12-03 16:49:01 +00:00
Ed Sanders a6147ffac8 Highlight comment after saving
* Init on wikipage.content hook
* Update page state variables after save

Change-Id: I05a3c766668999f05cfe06473652429025595196
2019-12-03 16:40:35 +00:00
Bartosz Dziewoński 872c73e1b2 Reserve space for the "Reply" link when hiding it
Otherwise the rest of the page may shift if hiding the link changes
line-wrapping. It felt super confusing when it happened to me while
I was testing an unrelated thing.
https://phabricator.wikimedia.org/F31254175

Change-Id: I53aecdbf3bfba579b48875532d251de0f1c81d6c
2019-11-25 16:23:00 +00:00
Ed Sanders a17fb49a68 Scroll ReplyWidget into view
Change-Id: I302346c85aacf9e410ff1468723b30fd04351032
2019-11-22 16:45:26 -05:00
Ed Sanders 87696c9c3c Move postReply code to controller
Change-Id: Ie66273d9b9f70b625ab7757c93b5884a01c70751
2019-11-21 08:24:57 -05:00