Commit graph

115 commits

Author SHA1 Message Date
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
jenkins-bot f1fc955dc3 Merge "Reply-to placeholder" 2020-03-03 22:59:44 +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 85b2cf00b1 modifier: Fix IE 11 incompatibility due to 'parentElement'
On IE 11, the 'parentElement' property is only supported on element
nodes, not on text nodes.

https://developer.mozilla.org/en-US/docs/Web/API/Node/parentElement#Browser_compatibility

There's no reason to use it here, 'parentNode' is the same for the
nodes we're concerned with.

Also remove the use in code adapted from MDN to avoid repeating this
issue in the future.

Bug: T246565
Change-Id: I0120feb3737c462f2a64e4ec084249a0fd57d0f0
2020-03-02 18:14:35 +01:00
Bartosz Dziewoński ea26009896 Work around mw.Uri crash on fallback encoding in links
Bug: T245889
Change-Id: I182f9ffa84a3b3cf4afafd536360572eda9d2714
2020-02-29 19:08:01 +01:00
Ed Sanders 7cb82b6862 Reply-to placeholder
Sets the placeholder text to "Reply to <user>".

Bug: T245227
Depends-On: I7f3a58b7093d00aace9f9c6a95a121ba4e901ad8
Change-Id: Ie51f1848c17bb892e7f64adf6f7f19fc38e56202
2020-02-29 17:29:07 +00:00
Bartosz Dziewoński 656b413b4b ReplyWidget: Disable "Reply" button if input is empty
Bug: T246058
Change-Id: Ib46f265eff3a4b4f59b5085ab33c5ebbef76003c
2020-02-29 03:58:28 +01:00
DLynch 2b21314671 Correct the integration for logging
Bug: T244874
Change-Id: I12d1a92f9a72b0ecd3fa7e2aa20289d1f57a46dc
2020-02-28 17:42:21 +00:00
jenkins-bot 9a1921488e Merge "Instrumentation: abort-navigate case" 2020-02-27 23:24:01 +00:00
jenkins-bot 0e22b4c79f Merge "Clean up the interface after discarding a reply" 2020-02-27 23:05:00 +00: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 e2bf0d25d2 Instrumentation: abort-navigate case
Bug: T244874
Change-Id: I0f8030e17d09d5b828732fc386b0b899f8c32564
2020-02-25 23:28:24 -06:00
Bartosz Dziewoński e9c401e3aa Ignore LRM and RLM before timezone indicator
They are not generated by MediaWiki, but they often appear when users
sign others' unsigned comments by copy-pasting the timestamp from the
history page.

Add test config data for nlwiki, exported by running this in the
browser console:

  copy(
    JSON.stringify( { wgArticlePath, wgNamespaceIds, wgFormattedNamespaces }, null, 2 ) + '\n' +
    JSON.stringify( mw.loader.moduleRegistry['ext.discussionTools.parser'].packageExports['data.json'], null, 2 )
  );

Bug: T245784
Change-Id: Icbcdc5a028e9ce2cb09173f87769e525ec3082fc
2020-02-25 00:20:00 +00: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 ff0386239f Only detect comments with real signatures
Consequences of this are visible in the test cases:

* (en) Tech News posts are not detected.
  Examples: "21:22, 1 July 2019 (UTC)", "21:42, 29 July 2019 (UTC)"

* (en) Comments by users who customize the timestamp are not detected.
  Examples: "10:49, 28 June 2019 (UTC)", "21:34, 14 July 2019 (UTC)"

* (en) Comments with signatures missing a username are not detected.
  This sometimes happens if a comment is accidentally signed with
  '~~~~~' (five tildes), which only inserts the timestamp.
  Examples: "17:17, 27 July 2019 (UTC)", "10:25, 29 July 2019 (UTC)"

* (pl) A lone timestamp at the beginning of a thread is not detected.
  It's not part of a post, it was added to aid automatic archiving.
  Example: "21:03, 18 paź 2018 (CET)"

Bug: T245692
Change-Id: I0767bb239a1800f2e538917b5995fc4f0fa4d043
2020-02-21 01:30:54 +01: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
jenkins-bot d8b0cf747f Merge "Blacklist/convert tables and headings in VE target" 2020-02-19 21:07:44 +00:00
jenkins-bot 8043769e5c Merge "Remove vertical padding from VE visual target" 2020-02-19 21:07:43 +00:00
Ed Sanders 5f966e845b Remove vertical padding from VE visual target
Compensates for vertical padding on paragraphs.

Match source mode padding to that of a TextInputWidget.

Change-Id: Ia53d8d2a6b9eff464c6c61152d02250088049bf9
2020-02-18 15:53:45 +00:00
Ed Sanders 97cf4e8440 Blacklist/convert tables and headings in VE target
Change-Id: I5e58fb1da1cb793bad5fb9640ef2dbf14c96d082
2020-02-18 15:44:40 +00:00
Ed Sanders 1a93e420b3 Sig preview: Increase opacity
Closer matches disabled-grey which is more accessible.

Change-Id: Ieb98e88bb42fdec1f1089919cf2c0c5f72b21877
2020-02-18 15:43:06 +00: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
Bartosz Dziewoński 3896babde3 Improve comment ruler drawing in debug mode when indentation is weird
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
2020-02-11 02:32:32 +01:00
Bartosz Dziewoński a491e3aaac Rename the lines denoting child comments in debug mode to "rulers"
I got tired of typing "relationship".

Change-Id: I318bdd1c049d7ab7fdeb1512a083cf8f4121d598
2020-02-10 22:14:38 +01:00
Ed Sanders c8564f6ccb Re-style preview
Bug: T238177
Change-Id: Iabc7cfa7595d60cbd0482340bd159002ee5a6b0e
2020-02-08 00:13:41 +00: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 601226199d Merge "Handle comments before first section heading" 2020-01-30 23:19:29 +00:00
Bartosz Dziewoński e29b8173bf Handle comments before first section heading
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
2020-01-30 00:14:46 -08: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
jenkins-bot 95a663ff62 Merge "ReplyWidget: Handle save errors" 2020-01-29 20:51:50 +00: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
Bartosz Dziewoński 30fcfec1fd parser: Merge multiple comments on one line
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
2020-01-22 02:21:43 +01:00
Bartosz Dziewoński 6d243ed74a ReplyWidget: Handle save errors
Bug: T240519
Depends-On: Ie18666b41f4aff1ab4bcf93f9df6e3000ac7b500
Depends-On: I2a731cb273401074e65f9283c1f629dbdb272002
Change-Id: Ice92fafb1f546510dab28e3f8aa7d2280668965a
2020-01-21 22:21:57 +01:00
Ed Sanders e83af3a4e9 Allow plain reply widget to grow without limit
Matches the behaviour of the VE widget.

Change-Id: If6540c4e91566da878c95b40e98ebe5f996125ce
2020-01-21 19:30:34 +00:00
Ed Sanders fe48688a76 Teardown the widget as soon as possible
Bug: T241393
Change-Id: I5978133637844fcad81af426465b6f16829ee9b3
2020-01-15 16:21:59 +00:00