Commit graph

92 commits

Author SHA1 Message Date
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 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
Ed Sanders cc5976e8db Fix <escape> handler again
Sepatate #teardown and #tryTeardown methods to make it
obvious what they do. Have <escape> call #tryTeardown
like the cancel button.

Change-Id: Ica0f3295bfee378bcd15d0b6a3ccea3c7917ad9b
2020-01-14 15:01:49 +00:00
jenkins-bot ad48d8d2f9 Merge "Option to integrate VisualEditor instead of textarea" 2020-01-09 18:08:18 +00:00
jenkins-bot 719bfee1b3 Merge "ReplyWidget: Load modules required by the content when previewing" 2020-01-08 23:28:21 +00:00
jenkins-bot 45e811a688 Merge "ReplyWidget: Pass 'title' when previewing" 2020-01-08 23:28:20 +00:00
jenkins-bot 88a467ec16 Merge "Fix re-initialization after page is updated" 2020-01-08 23:28:19 +00:00
jenkins-bot 8282cefb68 Merge "Fix crash when opening VisualEditor NWE while DiscussionTools enabled" 2020-01-08 23:28:18 +00: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
Ed Sanders c03cfde566 Fix 'escape' to teardown
Change-Id: Icc35ed83a1fa46ed592f617d0ff5dd60361c412d
2020-01-07 22:13:50 +00:00
Bartosz Dziewoński 25eb55000f Fix re-initialization after page is updated
I did this wrong in a6147ffac8, the
'dt-init-done' class was never cleared. Put it on another element.

Bug: T241861
Change-Id: I136bd9c12bcc80cff01f5d26a8a53524f0c533c6
2020-01-04 18:49:55 +01:00
Bartosz Dziewoński 0de5591889 ReplyWidget: Load modules required by the content when previewing
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
2020-01-02 16:00:03 +01:00
Bartosz Dziewoński d7aded339c Fix crash when opening VisualEditor NWE while DiscussionTools enabled
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
2020-01-02 14:44:18 +00:00
Bartosz Dziewoński e9b1037ec6 ReplyWidget: Pass 'title' when previewing
Bug: T241221
Change-Id: Ibabc32c9e5aeec6acec3bab9ad67d6ba3c4f27e8
2019-12-27 19:30:24 +01:00
David Lynch 5eea2d42fb ReplyWidget: calling undefined clear method
Caused by 49edbb82ab.

Bug: T240729
Change-Id: I23bf7fcb7e0265f9770629590702a07a3dbed2b1
2019-12-13 16:49:10 -06:00
jenkins-bot 2680307b37 Merge "Prompt users when pressing 'cancel' with content in the widget" 2019-12-13 19:00:41 +00:00
jenkins-bot b1d5e2953a Merge "Add beforeunload handlers" 2019-12-13 19:00:40 +00:00
Ed Sanders 49edbb82ab Prompt users when pressing 'cancel' with content in the widget
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
2019-12-13 18:15:48 +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 ccbbf386d2 Live preview
Change-Id: I1654e95e94686b27818cbc465d5a9e8600404634
2019-12-12 23:59:24 +00:00
Ed Sanders fc55b452b3 Show 'terms' message
Copied from Flow.

Change-Id: I5fe67948ba41a5a3364d8b0237583548c787d772
2019-12-12 22:26:22 +01:00
Ed Sanders 83ddc3d627 Show anon warning
Message copied from Flow.

Change-Id: Id003a1e57447b3087cd973964b3a88886d2cb7d6
2019-12-12 15:53:18 +00: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