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
This changes a vast number of test expected-outputs because it affects
the order of the keys when serialized.
Bug: T315400
Change-Id: I6ad2cded6ba7cb2cc5e5ba37ea60f4b18ecc26be
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
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
* 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
Change the HeadingItem constructor to take a 'null' headingLevel
and store this internally with the constant. Change the JSON
serializer to convert this back to null.
Change-Id: I27508eed75d94b99c5189548919309f8da7deb75
Since times immemorial, and for reasons lost to history, our test code
was adding an extra <div> wrapper before parsing the HTML used for
tests. This wasn't a problem, until now, because I want to add some
tests for T303396 that need to check that the *real* wrappers present
in some test cases are handled correctly.
Changes to test cases mostly remove a leading "0/" from serialized
ranges, corresponding to removing the extra wrapper.
Change-Id: Ia50e3590538c8cd274b02d2a937ba1a3fbb4ac89
It adds white-space between block tags and strips invisible tags.
It may be slightly slower (it takes HTML as input rather than DOM, so
we need to serialize the HTML first and then call it, rather than only
find and concatenate text nodes), but the difference is negligible,
and it seems better to use this method than to try to re-implement it.
Test runtime went from ~9.0s to ~9.5s locally, when testing using:
php tests/phpunit/phpunit.php \
extensions/DiscussionTools/tests/phpunit/ThreadItemTest.php \
--filter getText
Bug: T219138
Change-Id: I0cb89ebd2160e1ef499b78573c6688f493a4c42f
* We don't need it anywhere in JS, since we're not sending that HTML
to Parsoid.
* We only need it on the nodes directly containing our reply lines in
PHP, not all over the place.
Change-Id: I0a04388225f32654dda2f599442cd27a303b5d0a
The PHP DOM extension measures lengths and offsets in Unicode codepoints.
Our PHP code used UTF-8 bytes, causing some offsets to be slightly off.
Now it mostly uses Unicode codepoints as well (we're forced to use bytes
in a few places, because preg_match returns offsets in bytes).
In practice, this had no visible effect to the user. It caused the
markers `<span data-mw-comment-end="..."></span>` to be placed at
the end of their container instead of the correct position when the
timestamp contained multibyte characters (e.g. "ź" in Polish); but
the correct position is usually at the end of the container anyway.
In the test cases, the only difference is placing these markers before
a trailing line break inside `<p>...</p>` tags rather than before it.
The patch also accidentally fixes another bug, where element nodes
with no children (mostly <img>) were incorrectly excluded when calling
cloneContents(), because they were treated as if they were text nodes.
Change-Id: Iccdccf1078598f4b62cab96225e9c85a4c0e93ee
* ThreadItem::getText
* CommentItem::getBodyText (used when generating notifications)
* ThreadItem::getHTML (may soon be used in API)
* CommentItem::getBodyHTML (may soon be used in API)
* ImmutableRange::cloneContents (the common implementation for all
of the above)
The outputs are only lightly reviewed. This is mostly meant to
document the current behavior rather than the expected behavior,
to avoid making unintentional changes while refactoring.
Change-Id: I14471ee4969aa3d0b5577d9de2a6d4462fab4d09
We added it because the initial designs for the subscribe action were
much easier to implement like this, and topic "containers" (T269950)
would have required it.
However, the latest design of the subscribe action will not need it
(T279149), and topic containers are still very far away, so let's
remove it for now.
Bug: T280433
Change-Id: I21a23e9bea43f24d265750926fbd62b99038d3f1
The existing comment IDs can't be used to find the same comment on
a different revision or page (when it's transcluded), because they
depend on the comment's parent and its position on the page.
Comment names depend only on the author and timestamp. The trade-off
is that they can't distinguish comments posted within the same minute,
or in the same edit, so we will still need the IDs sometimes.
Prefer using comment names when replying, if they're not ambiguous.
This fixes T273413 and T275821.
Heading names depend on the author and timestamp of the oldest comment.
This way we don't have to detect changes to the heading text, but we
can't distinguish headings without any comments.
Bug: T274685
Bug: T273413
Bug: T275821
Change-Id: Id85c50ba38d1e532cec106708c077b908a3fcd49
Longer, but follows the style guide and less likely to conflict.
We need to account for init classes in the cache being around for
a while.
Change-Id: I738bc93393850db320fdbda2b003ca8ac40556da
The code we're testing already produces a string of serialized HTML,
no need to parse and re-serialize it.
Also, we recently learned that the precise format matters here
(T274709), and now this test *actually* covers the fix for that bug.
Follow-up to 5b26e9664b.
As a downside, this test might now spuriously fail if the format of
the output of Parsoid's XMLSerializer changes. Hopefully that won't
happen too often.
Change-Id: I69b514f545e47dcb437fb39a83edb8e2f19ed99b
Top-level comments that start or end with a list (inconsistent
indentation) would not have triggered the logic for detecting
wrappers.
Bug: T273692
Change-Id: Idcb4eed73e391f5f86eca2eb05cb3cea0d86f30a
We've recently decided that we want to "extend" comments until
the end of the paragraph (e36dc8e78a,
d0ae6c4e44).
However, we still had this special case that did the opposite: it
ensured that if a comment ended in the middle of a text node, the
comment would not be extended to the end of the node. Remove it.
Note the change in the test file signatures-funny-formattedreply.html,
which actually covered this case specifically.
Change-Id: Id1384bb0c6e1a5f0c70f55efcb4caa240f230f07
As CommentFormatter no longer needs HTMLFormatter, remove
the inheritance and make addReplyLinks a static method.
Testing locally this is marginally slower, going from 2.55s
to 2.9s for the CommentFormatterTest case.
Bug: T266317
Bug: T267973
Change-Id: If69749cae678a1647a138d782a32032189f55cec
After recent changes allowing ThreadItems to have IDs, they can now
also have warnings about duplicate IDs.
Bug: T267035
Change-Id: If3edfe34e6e29741e29fac8946a3c88badc4ab7f
Use the same logic for marking ranges in the document, and ensure
that the heading range does not include section edit links or
section numberings.
Change-Id: I782caafc34fee2a822b0a17b24dd6b9528202eca
We avoided fixing these because it causes changes in just about all of
the test data, which is annoying when reviewing or blaming changes.
But the previous several commits also caused changes in just about all
of the test data, so we might as well do this too.
Change-Id: I83b64d83b6f12c04dc06c0cadff7cdd89417e137
To avoid old threads re-appearing on popular pages when someone
uses a vague title (e.g. dozens of threads titled "question" on
[[Wikipedia:Help desk]]: https://w.wiki/fbN), include the oldest
timestamp in the thread (i.e. date the thread was started) in the
heading ID.
Bug: T264478
Change-Id: If918bfd5e025248923d1939bc86916697ead95a0
Sequential numbers aren't great because they change when an earlier
comment is archived. Parent comment/heading IDs should change less
often.
This also makes much more sense for disambiguating subsections,
e.g. a dozen identical ===Votes=== sections for a dozen proposals.
Bug: T264478
Change-Id: I466454984fd919ebef35f2b37ddb5d86dc842996
Our threads now also contain all replies to their sub-threads.
This is similar to how sections work in MediaWiki, where the parent
section also contains the content of all the lower-level sections.
We're going to need this for notifications about replies in a thread.
Bug: T264478
Change-Id: I241fc58e2088a7555942824b0f184ed21e3a8b6f
Previously, only comments could have IDs, because we only needed IDs
for replying. But we might also use them for notifications soon.
Bug: T264478
Change-Id: I1bcad02bf17ab54bc5028a959543c10f0430836b
