Commit graph

398 commits

Author SHA1 Message Date
Bartosz Dziewoński 4bbbdc9703 EventDispatcher: Try really, really hard to read from master
Follow-up to 37d6825c14.

Bug: T289717
Depends-On: I2a614915c7d9ffbc4f466204b2684478fb52f30e
Change-Id: Id2e040a19e457a2a8f4121b04ebd43bf6ea64181
2021-08-25 21:38:10 +02:00
jenkins-bot b1eb884966 Merge "Improve discussiontoolssubscribe API documentation" 2021-08-24 02:42:17 +00:00
jenkins-bot 5d9d28b280 Merge "ApiDiscussionToolsSubscribe: Remove redundant checks" 2021-08-24 02:41:26 +00:00
Bartosz Dziewoński 6dd90eda4a Improve discussiontoolssubscribe API documentation
Bug: T280314
Change-Id: I0987faac2cd8c5276a37869212fa968f0d642c09
2021-08-23 18:40:00 +00:00
Bartosz Dziewoński bb5054435f ApiDiscussionToolsSubscribe: Remove redundant checks
$user is guaranteed to be a User object.

Also, while we're here:
* Remove an unnecessary TODO. There's no need for a more specific
  error message, this one is just fine, other extensions use it too.
* Replace isAnon() with !isRegistered(). It'll probably be deprecated
  soon, because the new UserIdentity interface doesn't include it.

Change-Id: Ifbe98f4eccef79deee6bdb54c1bccf49807a9563
2021-08-23 20:39:18 +02:00
Bartosz Dziewoński 3e19fce67a Load styles in ParserOutput too
We load them in OutputPage, because we need that to handle the
query parameter 'dtenable=1', but do it here as well just in case
the pages are somehow shown without involving OutputPage.

Per code review:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/713681/1/includes/Hooks/ParserHooks.php#90

Change-Id: I3f287a9e146de7fd4d37c47dfa47eeb03eeb8cf1
2021-08-20 18:49:04 +02:00
jenkins-bot ccd2769b17 Merge "Remove remnants of the cookie hack for loading unavailable tools" 2021-08-19 16:32:10 +00:00
jenkins-bot 52ff6ef77e Merge "HookUtils: Simplify check for your own talk page" 2021-08-19 16:32:07 +00:00
jenkins-bot e2ef5764da Merge "Minor cleanups in topic subscription code" 2021-08-19 16:32:04 +00:00
Bartosz Dziewoński c8fa10770a Remove remnants of the cookie hack for loading unavailable tools
Follow-up to ee524d6bd6.

Change-Id: I1b4dc587e6b629b05ca0581dcd384a2bc7f01d2a
2021-08-18 20:45:02 +02:00
Bartosz Dziewoński 75d63bb056 Remove repetition in "Discussion pages" preferences
Bug: T289187
Change-Id: Id6c90546e4b57754e13b55f7c61610b51de87966
2021-08-18 20:20:34 +02:00
Bartosz Dziewoński 4aca61990a HookUtils: Simplify check for your own talk page
This approach is used in our new topic tool empty state code and is
much nicer. Also fix typo in a comment nearby.

Change-Id: I80755ef0960a172b0f370c36c1979a86498d6fa9
2021-08-17 22:22:15 +02:00
Bartosz Dziewoński bcd92a5d33 Minor cleanups in topic subscription code
For automatic topic subscriptions, I plan to introduce a third
subscription state to indicate them. This patch includes minor tweaks
I wanted to add while working on that:

* Introduce constants instead of numbers
* Remove a TODO that doesn't seem like a good idea any more
* Remove a `"length": 1` on sub_state that did not do anything
  (but it might have been meant to indicate that it was supposed
  to be a boolean, which would be wrong)

Bug: T284836
Change-Id: I6e6096968ad38510102287bccd349090b6ca4280
2021-08-17 22:22:15 +02:00
jenkins-bot 781992de2d Merge "Create a hidden revision tag for talk page comments" 2021-08-17 02:07:10 +00:00
jenkins-bot e090c455cc Merge "EventDispatcher: Fix ignoring level 3+ headings" 2021-08-16 22:08:42 +00:00
jenkins-bot 2cd97c4be5 Merge "Handle highlighting and scrolling to comments for bundled notifications" 2021-08-16 20:43:37 +00:00
Bartosz Dziewoński db28a3d3a7 Handle highlighting and scrolling to comments for bundled notifications
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
2021-08-16 22:03:30 +02:00
Bartosz Dziewoński ad04b24ffd Create a hidden revision tag for talk page comments
Bug: T262107
Depends-On: I21159d03eebaf46ad94f4273ba698a59b8019185
Change-Id: Iceddfaf6a4bcc5e8b5c85c8cd5638bf14aa7db03
2021-08-16 15:42:51 +00:00
Bartosz Dziewoński 47510a22f3 EventDispatcher: Fix ignoring level 3+ headings
The code (prior to d25825a754) assumed
that level 3+ headings would always follow a level 2 heading or the
placeholder heading, but we don't generate a placeholder heading if
there are no comments in section zero.

Add more tests to confirm that comments under level 3+ headings (that
are not sub-headings of level 2), and level 1 headings, are ignored
when generating notifications, and do not mess with normal headings.

Bug: T288775
Change-Id: Ic57b56752a4797cb01234f66e0ed7b849752bd70
2021-08-16 15:42:06 +00:00
Bartosz Dziewoński d25825a754 EventDispatcher: Remove failing invariant check
To be investigated in the future.

Bug: T288775
Change-Id: Ic27418a0ec976347be5fa586bbd32cc4a0d8d511
2021-08-12 22:37:51 +02:00
vladshapik 613b0a9b27 Avoid using deprecated ParserOptions::getUser
Bug: T287858
Change-Id: I13ef6ef128a8316f699c6e038adf82d18bf81b96
2021-08-10 16:43:11 +03:00
DannyS712 88ba997bed Injected SubscriptionStore into UnsubscribeAction
Dependency injection is now available to actions,
extension already requires MW 1.37+

Bug: T253078
Change-Id: I473abac19ed5e6f3c6706797e91704ff635f64c6
2021-08-09 22:38:52 +00:00
Bartosz Dziewoński b46893eb7d Remove pointless uses of preserveWhiteSpace property
This DOMDocument property has no effect, because we do not use
DOMDocument methods for parsing HTML, but rather DOMUtils::parseHTML()
provided by Parsoid.

Change-Id: I1d9e73e53f2d44f41cf9dcda4f06ac8647671096
2021-08-09 23:45:48 +02:00
David Lynch 93f2e64f3f Only show the empty state on talk namespaces
This should only affect some edge cases like the project namespace (e.g.
Wikipedia:Village_pump isn't a talk page, but does get discussiontools.)

Bug: T288317
Change-Id: I509101063a1d64d09cff71a84bf48c69ab7a8c08
2021-08-06 16:04:23 -05:00
jenkins-bot 10c23d0eb1 Merge "Deal with document body consistently" 2021-08-06 03:08:28 +00:00
jenkins-bot 02c09106ab Merge "Remove use of DOMXPath to remove Phan suppressions" 2021-08-06 03:05:13 +00:00
jenkins-bot 125cb495e2 Merge "ImmutableRange: Remove Phan suppression" 2021-08-06 02:47:07 +00:00
jenkins-bot 2fa030aafc Merge "Apply an empty-state to pages with the new topic tool enabled" 2021-08-05 16:52:49 +00:00
David Lynch 91af0594b5 Apply an empty-state to pages with the new topic tool enabled
This includes the dtrepliedto URL functionality from
I3f81e4d77faed367606e47678b8896051982359d.

Bug: T274831
Bug: T274832
Bug: T277329
Change-Id: I035d04f30c8312b0cb42902d3bf940df1482ffb3
2021-08-04 18:46:28 -05:00
jenkins-bot d14f31fd08 Merge "Improve notifications for comments posted in close succession" 2021-08-03 23:17:55 +00:00
jenkins-bot d4f4e49c7e Merge "Allow the new topic tool to handle URLs like action=edit&section=new" 2021-08-03 16:14:21 +00:00
Bartosz Dziewoński 8de8d80cde Deal with document body consistently
Use `DOMCompat::getBody( ... )` as a nicer getter than
`->getElementsByTagName( 'body' )->item( 0 )`.

Remove overly defensive checks and redundant annotations on its
return value. Since we're dealing with HTML documents throughout,
the document body is guaranteed to exist.

We previously needed some of them to convince Phan when it thought
the body may be null, but this seems to no longer be needed.

Change-Id: If7aee7b6adbfa78269c7ba28b26a6eaa21fe935b
2021-08-03 15:12:55 +02:00
jenkins-bot 1f4706a308 Merge "Recognize links to add a new topic that use Special:NewSection" 2021-08-02 17:42:30 +00:00
Bartosz Dziewoński 7a9fd40eb9 Remove use of DOMXPath to remove Phan suppressions
Use DOMCompat::querySelectorAll() instead.

CommentModifier::isHtmlSigned()
* Copied the CSS selector from the JS equivalent function.
CommentUtils::unwrapParsoidSections()
* Copied the CSS selector from the JS equivalent function (in VisualEditor).
CommentItem::getMentions()
* Trivial.

This causes Phan to report some more issues, which are also fixed.

Follow-up to 25272e7a4a.

Change-Id: Iaf1222f7114916f2eca19942c3686168899486fd
2021-08-02 18:23:16 +02:00
Bartosz Dziewoński 8b73612585 ImmutableRange: Remove Phan suppression
We can just use insertBefore() normally. There was never a PHP bug,
but rather a Phan bug, and it no longer affects us.
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/596813/70/includes/ImmutableRange.php#373

This reveals a bug in CommentParser where it sometimes produces
incorrect ranges (we were incorrectly treating `false` like `null`,
hiding the issue). I'll fix it in a separate commit.

Follow-up to 25272e7a4a.

Change-Id: I4afba38f1d82ddbf8732bfe3e4d4f6ebe2f8de5d
2021-08-02 17:51:26 +02:00
Bartosz Dziewoński a5099739a6 Improve notifications for comments posted in close succession
In case 4 and case 6, no notifications are expected. In all other
cases we now get the expected notifications.

Bug: T285528
Change-Id: I9e813bb3a053bc1232783f9eae1ad75672b4fa7e
2021-08-01 12:27:33 +02:00
C. Scott Ananian 25272e7a4a Don't refer directly to PHP dom extension classes; avoid nonstandard behavior
These changes ensure that DiscussionTools is independent of DOM
library choice, and will not break if/when Parsoid switches to an
alternate (more standards-compliant) DOM library.

We run `phan` against the Dodo standards-compliant DOM library,
so this ends up flagging uses of non-standard PHP extensions to
the DOM.  These will be suppressed for now with a "Nonstandard DOM"
comment that can be grepped for, since they will eventually
will need to be rewritten or worked around.

Most frequent issues:

* Node::nodeValue and Node::textContent and Element::getAttribute()
can return null in a spec-compliant implementation.  Add `?? ''` to
make spec-compliant results consistent w/ what PHP returns.

* DOMXPath doesn't accept anything except DOMDocument.  These uses
should be replaced with DOMCompat::querySelectorAll() or similar
(which end up using DOMXPath under the covers for DOMDocument any way,
but are implemented more efficiently in a spec-compliant
implementation).

* A couple of times we have code like:
  `while ($node->firstChild!==null) { $node = $node->firstChild; }`
and phan's analysis isn't strong enough to determine that $node is still
non-null after the while.  This same issue should appear with DOMDocument
but phan doesn't complain for some reason.

One apparently legit issue:

* Node::insertBefore() is once called in a funny way which leans on
the fact that the second option is optional in PHP.  This seems to be
a workaround for an ancient PHP bug, and can probably be safely
removed.

Bug: T287611
Bug: T217867
Change-Id: I3c4f41c3819770f85d68157c9f690d650b7266a3
2021-07-30 18:15:40 -04:00
C. Scott Ananian 5203d30ea6 Use DOMCompat::newDocument() to create a new Document
For compatibility with Parsoid's document abstraction (Parsoid may
switch to an alternate DOM library in the future), don't explicitly
create a new document object using `new DOMDocument`; instead use
the Parsoid wrapper `DOMCompat::newDocument()`.  This ensures that
the Document object created will be compatible with Parsoid.

There are a number of other subtle dependencies on the PHP `dom`
extension in DiscussionTools, like explicit `instanceof` tests; those
will be tweaked in a follow-up patch
(I3c4f41c3819770f85d68157c9f690d650b7266a3) since they do not affect
correctness so long as Parsoid is aliasing Document to a subclass of
the built-in DOMDocument.  Similarly, the Phan warnings we suppress
do not cause runtime errors (because of the fixes included in
c5265341afd9efde6b54ba56dc009aab88eff83c) but phan will be happier
once the follow-up patch lands and aligns all the DOM types.

Bug: T287611
Depends-On: If0671255779571a91d3472a9d90d0f2d69dd1f7d
Change-Id: Ib98bd5b76de7a0d32a29840d1ce04379c72ef486
2021-07-30 18:15:11 -04:00
Bartosz Dziewoński cfbf437d9d Allow the new topic tool to handle URLs like action=edit&section=new
Depends-On: Ib9302e2fda7dadf1edc43c0107db7234eb4bdf7a
Depends-On: Ic7dd677ea219938969f60bab91387c2e03ebdbe6
Bug: T282204
Change-Id: I2e7b2682da7beb3c1c469bb784a9a8ec3998aeb9
2021-07-30 09:25:21 +02:00
Bartosz Dziewoński d0e4aeaecb Fix notifications when new comment is under subheading
The user interface only allows you to subscribe to level 2 headings.
But we would generate events for whatever heading was the closest,
If it was e.g. level 3, no one would receive that notification.

Now we generate events for the closest level 2 heading, or we don't
generate the event at all if there isn't one (if the only headings are
of level 3 and below, or level 1, or if the comment is added before
the first heading on the page).

Bug: T286736
Change-Id: Iae99853070e353ab81c9cc29ef1d53c877adfc66
2021-07-24 05:28:10 +02:00
Bartosz Dziewoński 801b57b0f4 Add PHPUnit integration tests for EventDispatcher
Bug: T286608
Change-Id: I711483be80d455f4439e96d37844ee4552619a92
2021-07-24 05:28:04 +02:00
Bartosz Dziewoński 4ebf05d802 Recognize links to add a new topic that use Special:NewSection
Bug: T277371
Change-Id: I40a13d8bf87bcd3ecea1427b444b6b7b621213c4
2021-07-22 22:25:11 +02:00
libraryupgrader b0884b177c build: Updating dependencies
composer:
* mediawiki/mediawiki-codesniffer: 36.0.0 → 37.0.0

npm:
* postcss: 7.0.35 → 7.0.36
  * https://npmjs.com/advisories/1693 (CVE-2021-23368)
* glob-parent: 5.1.1 → 5.1.2
  * https://npmjs.com/advisories/1751 (CVE-2020-28469)
* trim-newlines: 3.0.0 → 3.0.1
  * https://npmjs.com/advisories/1753 (CVE-2021-33623)

Change-Id: I7a71e23da561599da417db3b3077b78d91173bbc
2021-07-22 16:29:04 +00:00
Bartosz Dziewoński 4ebe4b29bb Only show [subscribe] links on sections that contain at least one comment
Bug: T285796
Change-Id: I48264d55464fa6bce56b47fc1075f5d348101676
2021-07-13 02:35:15 +02:00
Bartosz Dziewoński 37d6825c14 EventDispatcher: Ensure we fetch page content from the primary database
We used an internal API requests to fetch page content because it was
easy, but there's no way to guarantee that it returns data from the
primary database.

Use ParserOutputAccess::getParserOutput() to fetch from cache if
available. Also, use canonical output instead of user-specific,
not that it should matter.

Bug: T285895
Change-Id: I7dcd9659be77746dc2a0c4eeae2319887936b555
2021-07-01 14:52:13 +00:00
jenkins-bot f8f776d4e7 Merge "CommentFormatter: Remove old cached compatibility code" 2021-06-29 22:32:54 +00:00
David Lynch ae0aeff3a3 Allow dtenable=0 to disable DiscussionTools
Bug: T285578
Change-Id: Ic722285d51ea1a51593ab63fac516b4f992eb1c8
2021-06-29 21:20:35 +00:00
jenkins-bot b6b60bbe0b Merge "Config option to enable topic subscriptions backend and dtenable=1 URL parameter" 2021-06-29 15:21:09 +00:00
Bartosz Dziewoński a38eb4fe18 CommentFormatter: Remove old cached compatibility code
Added in 4bbfe6cb5d.

Change-Id: I4a6b265897beb60abc2a7c1e6bfc4acc646fda56
2021-06-24 15:38:02 +02:00
Bartosz Dziewoński 1517e2e094 Remove unused RL module
Unused since 6a24ceaeca.

Change-Id: I96c8a7d7f7527e86ed728e72eb7232406aeb6942
2021-06-15 12:34:18 +02:00