Better describes that we are checking the editor used to make
the edit, rather than descibing some virtual "location".
Change options to 'discussiontoolsapi' and 'any'.
Change-Id: I3024517e70ed61c738e4bf46a2ac7b58c975d98a
According to Daniel it only worked by accident, and stopped working
after de63ad823abe:
getPageByReference() used to do an opportunistic lookup by ID when given
an instance of PageIdentity -- which is correct for EventDispatcher,
but problematic in the general case, causing T296063.
The correct thing to do here is to use getPageById(), since the canonical
association between revision and page is by page ID.
Bug: T297431
Change-Id: Icc1df0c9ca5345e65ef5f8daf0815013d7db0943
The 'legacyPrimary' links will take you to the section
the comment is in and should be used when you don't have
access to comment IDs.
Bug: T296018
Change-Id: I944feb90e7c3a69f81366f42fa110c58cac26dbb
* Add a N/A value for edit counts from anonymous users
* Only oversample with $wgDTSchemaEditAttemptStepOversample if the edit
is from DiscussionTools
* Consider $wgWMESchemaEditAttemptStepOversample for oversampling
Bug: T286076
Bug: T295995
Depends-On: Ieb3f6c6e1775c1ef53747c37003b17e3634d1c44
Change-Id: I91245a61dfbde8b5ec9b2893b9170cc4d73f7b0a
If the user talk edit or mention coincides with exactly one new comment:
* Change the primary link to be a direct link to the comment
* Add a text snippet to notifications that don't already include one
(user talk edits that are not new sections).
This is done for all such notifications, regardless of whether anyone
has topic subscriptions enabled.
Bug: T281590
Bug: T253082
Change-Id: I98fbca8e57845cd7c82ad533c393db953e4e5643
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
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
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
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
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
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
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
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
…without making the topic subscriptions feature available in user preferences.
Follow-up to these commits, which added these checks in ad-hoc ways:
* 9420f22e9d
* f3422f40a6
* 23a490deca
* a555db7892
Bug: T284491
Change-Id: If2e3fb1e06d1cc489fbca14796ed77c83bb52991
If the revision from which we generated the notification has been
deleted, we shouldn't include the content snippet, nor the direct link
to the comment (because the fragment ID is generated from the content).
This matches how Echo handles mention notifications.
Change-Id: Ica939f3a4efd39d0c295511d58280d3f9d584129
As it happens, most of Echo does not actually parse this message,
but it is for some reason parsed in HTML email notifications.
Change-Id: I414cd242d9bcc4d8b5a1c2a2a71be9e5f00ea8be
We don't display [subscribe] buttons on your user talk page,
but the API still allows those subscriptions.
Use the same approach as for mentions to ensure this doesn't cause
duplicate notifications.
Remove some code in SubscribedNewCommentPresentationModel,
now guaranteed to be unused.
Change-Id: I99a276a48d8562552ed2c54cc0323e8e428845fd
