ParserOutput::getTOCHTML() is being deprecated, and all skins are now
generating the TOC from the TOCData returned by
ParserOutput::getTOCData(). The SHOW_TOC flag was introduced in core
to determine if the TOC should be shown, but Vector-2022 *may* begin to
use other heuristics to determine whether to show the TOC (T315862).
We're conservatively going to process the TOC as long as there is
TOC present to process.
Bug: T328072
Change-Id: I38b439c6752157dbee9b09c9f5443a740dbaabf4
Otherwise, when a transclusion covers a section boundary, both whole
sections are considered to be transcluded.
We already use this method everywhere else (by way of
HookUtils::parseRevisionParsoidHtml), and VisualEditor also applies
the same transformation (mw.libs.ve.unwrapParsoidSections) before
opening the page for editing.
Bug: T327704
Change-Id: I9d8288e2740d816edb9cbc01d7e5642d52c610d3
Change code to match the documented consensus formed on T321683:
https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Exception_handling
* Do not directly throw Exception, Error or MWException
* Document checked exceptions with @throws
* Do not document unchecked exceptions
For this extension, I think it makes sense to consider DOMException an
unchecked exception too (in addition to the usual LogicException and
RuntimeException).
Depends-On: Id07e301c3f20afa135e5469ee234a27354485652
Depends-On: I869af06896b9757af18488b916211c5a41a8c563
Depends-On: I42d9b7465d1406a22ef1b3f6d8de426c60c90e2c
Change-Id: Ic9d9efd031a87fa5a93143f714f0adb20f0dd956
The alias stored in the language files is with underscores,
but the value is compared in CommentParser against text with spaces
Affected languages: bjn, hu, id, jv, kaa, tl, tpi, vi, war
Bug: T327021
Change-Id: I8626627d10a240973e631e24508937a9eee9fb14
We need to be careful about flooding the parser cache with parsoid
content. For this reason, we currently only write to PC for a certain
sample of edits. This logic is implemented in core in
ParsoidHandler::allowParserCacheWrite and controlled by the
TemporaryParsoidHandlerParserCacheWriteRatio setting.
DiscussionTools triggers parsoid PC writes when handling the
RevisionDataUpdates hook, so it should use the same sampling.
Change-Id: Ic33f57b10ae53f431a3c3484c4853e88bf80f47a
Test ServiceWiring.php using tests copied from CentralAuth. Because
phpunit does not support marking a file as covered, the ServiceWiring
file is ignored for code coverage as the tests fully cover the file.
Change-Id: I7da8d74fec84a5aa9c77bc0678ad8f55b550893a
We are seeing a lot of parser cache writes coming from
parseRevisionParsoidHtml. We should find out what is causing them.
Change-Id: I25440e0d759e19cc9769404beb6911c64d37d3e3
* Detect comment separators at the end of comments too
* Consider TemplateStyles associated with ignored templates
This unexpectedly improves a lot of cases other than T313097 too,
mostly where <br> or {{outdent}} was used within a paragraph:
splitting comments that were previously jumbled together, or restoring
content that was previously ignored for apps / notifications.
Bug: T313097
Change-Id: I9b2ef6b760f2ffd97141ad7000f70919aeab7803
Treat non-talk pages as empty ContentThreadItemSet objects, instead of
returning early from execute().
Exclude non-talk pages even if only the rev_id is specified.
If both rev_id and title are specified, use the title from the
database.
Bug: T325477
Bug: T325598
Change-Id: I3947ac94bb7c9a3d24b73c95f0df2cf847c955f2
The parser cache for parsoid output isn't yet ready for full load.
Don't flood it when running batch operations.
Change-Id: I77f3de30b0500f0e5c593f4d31dceef7720f848e
New config: DiscussionTools_visualenhancements_reply_icon_languages
Config is set up with a provide_default merge strategy so we can remove
items from it quickly if need be.
Bug: T323537
Change-Id: Ib748897a2162bb233000f7364e30b268932f4c4a
Used by MobileFrontend in I78cfb22fbe7d to prevent sections from
auto-expanding.
Bug: T321618
Bug: T322628
Change-Id: I6dafd5b9cb170bfa57f185849db6450162173399
Call ParserOptions::setRenderReason to allow us to track why we render
and in particular, why we write to the parser cache.
Change-Id: If42f802f4cf2da39b06cbb8a30c4dc7d9a663001
If the final content on a page is a heading, there would be an error as
we tried to access nextSibling on a non-existent node.
Also tidies up the case where there's an empty section that's not the
final section. It would have `othercontent` set to an empty string,
which was pointless -- the empty `replies` field is sufficient.
Bug: T321317
Change-Id: Ia58e97214e715c1f6b02c2e045d13f2df7393b80
Inspired by this Wikitech-l discussion:
https://lists.wikimedia.org/hyperkitty/list/wikitech-l@lists.wikimedia.org/thread/NWXPNHRNLEVXHSWX33H473OAWQP6CDOA/
To keep this simple for now, I am only removing redundant PHPDoc
comments on constructors, and only when all the documentation for
parameters completely duplicates type hints.
More could be done, but that can happen later when we have better
tooling. Redundant comments on constructors that take a dozen services
are by far the most annoying for me and I want them gone now.
Change-Id: I86cbf7d6e48035cfa06f780c8fb1b02e68709a0c
Also fix a CSS selector to handle content added in multiple
'OutputPageBeforeHTML' hook calls.
Bug: T323376
Bug: T323833
Change-Id: I480d9bf544d61f0cb7bfd04cadfbf053e7e1b70e
After recent changes (I101c1e84739a2ac1f562f2f7bdc4b8f53d9f3b23 and
Ifbde590ccb6bf3203a2f664cb0d8a73b8d507b78) these methods became
basically the same.
Change-Id: Iedc201e798a5a34713296b20b97ae6cc8b991b66
ApiDiscussionToolsPageInfo and ApiDiscussionToolsCompare in direct parsoid
or VRS modes tries to fetch HTML using VisualEditor thus stashing the
HTML gotten which we don't want, we only need it for viewing in these cases.
This seems like something that was/is already happening in RESTBase. So for
APIs in DiscussionTools that need the HTML for viewing, just get it from
parser cache and not stash it.
Bug: T323357
Change-Id: I101c1e84739a2ac1f562f2f7bdc4b8f53d9f3b23
Ideally this would just not run the hook for any interface messages, but
that condition isn't obviously available.
Bug: T316175
Change-Id: Ibd354eb7a0fb7a316dcbf09e64b80f2d9b4008c8
(Also fix some related CSS that was accidentally moved in
Ie5198e902ec3fa7a7eba56cef6c6f0ef71ef7314)
Bug: T323241
Change-Id: I1fa67965a1b6b827c500a9de63f5b5295bee840d
When a comment almost exactly matches the range of an
accidental complex transclusion consisting only of
pages from the 'Template' namespace and wikitext fragments,
I think we can safely allow replying to the comment.
Even if this turns out to be incorrect in some cases,
the failure will be more graceful after the changes in T313100:
instead of potentially duplicating contents from a template,
the worst case now is that the reply will appear in the wrong
place (at the end of the transclusion).
Bug: T313093
Change-Id: Ie8da09d74a652d893fd8c3e2435ef6cb70fad64a
We wrap a `<div>` tag around the `<h2>`, and move some elements there.
The markup is inspired by and compatible with my proposal for T13555.
The "ext-discussiontools-init-section" class is moved to the `<div>`.
A small patch is needed in MobileFrontend to preserve the section
collapsing functionality: I11bff21e81046898ca63f3f432797129fa70ad88.
The following elements are now outside of `<h2>`:
* Metadata bar
* Subscribe button
* Ellipsis menu (only shown on mobile)
The following elements are sadly still inside of `<h2>`:
* Subscribe links (only shown on desktop)
* Section edit links from MediaWiki core
Trying to move them mucks up the CSS too much. I hope we can resolve
this later as a part of the work on T13555.
Depends-On: I11bff21e81046898ca63f3f432797129fa70ad88
Bug: T314714
Change-Id: I0bbdcfa02c334858737855349d7a35746de1d8f2
Use revision IDs to break the tie, consistently with MediaWiki (see
RevisionStore::getRelativeRevision) instead of assuming that the
revision we're processing now is somehow both older *and* newer than
the other one (the mind boggles how that ever made sense to me).
Change-Id: I9f1a07124301a36be68578d908353b72f0442c00
isFeatureEnabledForOutput already checks if the mobile flag is enabled,
but it also respects the dtenable=1 override.
Change-Id: I95035281bf301b22c1a9ef4c06ec54cdd0cbc85c
We originally used 'templates' because it seemed like an obvious
choice for HTML files, and because 'packageFiles' requires extra code
to include anything that isn't a .js or .json file.
However, the templates are expected to be HTML fragments rather than
whole documents, and they are parsed in a particular way that takes a
lot of code to clean up (which we needed to do, because we use the
same test files for testing PHP code).
I tried doing it in the 'packageFiles' way, and the extra code doesn't
seem that bad in comparison after all. Moreover, the 'templates'
mechanism (when used the intended way) feels vaguely deprecated in
favor of Vue.js, and I'd rather move away from it.
This makes the tests faster too (probably mostly thanks to the removal
of the clean up code) – on my machine they go from 1800ms to 1500ms.
(Simplify linearWalk tests, as we no longer need to do weird things
with document fragments to get consistent outputs in PHP and JS.)
Change-Id: I39f9b994ce5636d70fea2e935a7c87c7d56dcb26
Part of my secret plan to delete ApiParsoidTrait.
* Inject RevisionLookup into ApiVisualEditor
* Use RevisionLookup::getRevisionById instead of ApiParsoidTrait::getValidRevision
* Use RevisionLookup::getRevisionByTitle instead of ApiParsoidTrait::getLatestRevision
* Use standard MediaWiki error messages
Change-Id: I7244ee4916fb011fad5faa1d9f837e83f6ac2dc1
Follow-up to I8cf8b6960533718646189263acabc852ea976416, where the ...
was unintendedly removed.
Also switch to a suppression, because the type should be documented in
ParsoidClient, rather than enforced in callers. We will remove the
suppression once the documentation is updated.
Change-Id: I3ee2534959c8375d29f43e8391894f0a2002ae1c
Follow-up to d0126ce6de which made them
default-on for all mobile. These two taken together mean that the
mobile visual enhancement features now *only* depend on this config,
rather than on whether the individual features are enabled on desktop.
Bug: T318871
Change-Id: If767753e6d33f19bbc540d4e74273e478198388c
The GlobalPreferences extension would remove disabled fields from
the form, avoid referring to it in the disable-if param.
Bug: T317526
Change-Id: I6ec177a1f20855c90eb4ea17bb6625c75e5b9617
We can't use these is class method function signatures, but we
can use within the functions themselves.
Change-Id: Ic24e47d6647226172a3bfacd81398d26143d98e4
Will fallback to MainConfig gotten from services in VisualEditor.
VE will remove this complete soon.
Depends-On: I787c0afb227308aab56770d14d62e08eb0084a6c
Change-Id: I4b9e7362a1bf1b4f224c8c652d40851a18c19824
* Make "X comments" appear on a new line deliberately
* Remove parentheses around "X comments"
Bug: T309463
Change-Id: I803eee9db59c633f129f15e436242a12bcc627f0
getVal/getText can be expensive because they do Unicode normalization.
We can skip this when we know we aren't going to do anything meaningful
with the value anyway.
Change-Id: I9d939a44df6b67bcd8429096d89600ce1566ca39
The config is currently unused in some classes, but this is okay since
it might be used in the future.
Change-Id: Ie25fc52cc5d3476c9445e182975d229991316bd3
The implementation in Parser doesn't descend into sub-thread.
Re-use the getThreadSummary method in ThreadItem and traverse
the thread properly.
Bug: T298617
Change-Id: I318d9012eb83f37ccbe463923524ef2e9f995ced
This changes a vast number of test expected-outputs because it affects
the order of the keys when serialized.
Bug: T315400
Change-Id: I6ad2cded6ba7cb2cc5e5ba37ea60f4b18ecc26be
In the future the notifications can be improved to look up
the new location of the comment, using the permalinks data.
Depends-On: Ia8a21749a8edc20f34b2a3e445278ea6922b9109
Bug: T299657
Change-Id: I5f5e7b73fb84ff0d31fb8260b24066a17da71628
We can no longer change IDs so easily, because they're stored in the
permalink database, so remove this mechanism to make sure it's not
accidentally used in the future.
Change-Id: I392ee1f49c48fc2f23d05e9a37c643438b4f2b9a
We didn't correctly reconcile the changes from
Ie0412b9f238d5ff8e54fd2ea358c1c26e303f4e1 and
I419883b5f9a4291b4bf575d57195d553fd5e291e.
Bug: T315821
Change-Id: I5a93cbb3bf029b352e808cbd9cb1ea3286e20c94
This code is throwing exception for as yet uncertain reasons, which
may cause other updates to not be executed (e.g. Echo notifications).
Put a try…catch around it while we investigate.
Bug: T315383
Change-Id: Ic7aba32369f69c2e8165d5d6d25687a4cb6e0be8
From in-person code review with Amir:
* getConnectionRef() has been deprecated in favor of getConnection()
* 'watchlist' query group no longer exists (T263127)
* Since this simplifies things, we can remove our wrapper methods
Change-Id: Ic610233b2e6d6ed68f7e1f5b31bb8996ed77f04b
From in-person code review with Amir: using tableExists() is a bad
idea, because the table might not consistently exists on all replicas.
It's better to have a config option despite the inconvenience.
Bug: T315353
Change-Id: I728759634c454c0dcbdc4603c15cab60415c7c03
Move the code that generates these wrapper nodes and attributes
from postprocessReplyTool() (only called when reply tool is enabled)
to addDiscussionToolsInternal (always called).
This undoes some changes from 31c57d594a and 980b2c38bc.
Bug: T314707
Change-Id: I07ed210375d494047670015410430c087d67f21a
It turns out that using the "enableSectionEditLinks" post-cache
transform option was not a good idea, as it is also set when viewing
old revisions and in some other cases.
However, in the pre-cache parsing, we have access to getIsPreview(),
which is exactly what we want. I think we can safely do this there.
We were already using that prior to 2bc76dabd7.
Bug: T314260
Change-Id: I7f769db48eff9fa434483902a4b5ac2f5fc96b3d
We might want to delete the code that defines it in the future.
The core message is the same in most translations, and available in
100+ more languages.
Change-Id: I230e051940fdd7b89989453eccbffac804a7ddea
Rather than setting it on both the reply link and the reply button,
set it on their parent element.
Update ReplyLinksController to handle this.
Change-Id: I650e9c0ebd354a82b8f66a63c5b4c02b2e29b105
This improves the behavior when replying to these comments
and the message snippets shown in notifications.
Bug: T313097
Change-Id: Ia10400472c9e999fa526c7437a03b72461c37b74
Add the redirect check to shouldShowNewSectionTab.
Try to run cheaper checks before more expensive ones.
Depends-On: I5755863243d8ad336ad20626f439d70eb3b31f32
Bug: T312599
Change-Id: I6848e529a2537d4058613db0c3b900bc9f4f59f8
Othercontent would often contain the opening tag of the next heading /
section. By looking for the closest node with a previousSibling we can
more-reliably escape the heading.
Also, only add the initial placeholder if there's content before the
first heading. We do this by testing for any siblings before the
startContainer of the first heading -- if there are any, assume this
means there's some sort of content. (This can still result in a
placeholder with `othercontent:""` if there's only whitespace before
the first heading.)
Bug: T313850
Change-Id: I080205b74413c46d3cf3442e79276145aaa9439c
We had different logic for the empty state on pages that exist and
those that don't. For the second case, we relied on the new topic tool
becoming unavailable in some cases, but that could cause complications
elsewhere (e.g. if the page also had a custom button to add a new
section).
The new topic tool is now available regardless of the page. Instead,
the empty state is controlled separately, by the same method in both
cases.
As far as I can tell, the effect is exactly the same as before.
Change-Id: I5b36e6f027cf76dd0e3a8ee3cb5156fe1eaac8a8
Remove hacks marked as 'TEMPORARY' now that we consider the
HTML to be stable enough.
This will avoid issues with HTML/CSS being out of sync in future deployments.
Bug: T313560
Change-Id: Ie00ff38a422f241add19d500adaf22dfeee10e8c
* clock, userAvatar, speechBubble were dropped before topic
containers was merged
* Load ellipsis & edit icons on mobile only
* Load bell & bellOutline for topic subscriptions button
Change-Id: I77d1336627b564be756e3ec50b4686b8f9ac72dc
