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
