I was curious why running some of the PHPUnit tests in this code base
takes so long. While I could not spot an obvious bottleneck I found
a lot of code that is extremely hot, e.g. called a hundred thousand
times. A few obvious optimizations are possible in this code, e.g.
not calling the surprisingly expensive DOMCompat::getClassList
multiple times.
Change-Id: If22bbc1aedd2c36db1ab2343de5737009050b7bb
Since 92f5cfd8 we support "mw-notalk" to suppressing comment detection
in pages or sections.
Until now, it only worked when the comment timestamp was surrounded by
a marked element. However, when a marked element was directly adjacent
to a comment, it would sometimes become a part of the comment range.
This can no longer happen now.
Existing use cases for this were the {{outdent}} and {{tracked}}
templates, which we handle specially since 50ad5bb2 and ddd391b6.
It's a bit ugly to hardcode specific templates like that, and this
provides a better solution for the future. The added test case
displays some other potential uses.
Bug: T324132
Change-Id: I7ffd299ef5957b35da8d01f9a0ed5a7a9a78be83
The following are now available in all browsers we support:
* mix-blend-mode in CSS
* Node#contains
Change-Id: I324948f0bceac6d86999300e2b2c8756613e56d2
* 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
This improves the behavior when replying to these comments
and the message snippets shown in notifications.
Bug: T313097
Change-Id: Ia10400472c9e999fa526c7437a03b72461c37b74
Also fix a bug where headings would be ignored while checking for
comment frames. See task for detailed explanation.
Bug: T303396
Change-Id: I6495826b4b050ea80680e0798ac6ab4497a7c09e
Phan can analyze them now and reports some issues with types.
* Add some assertions on types where we're sure that we're using an
Element or non-null, but Phan can't prove it
* Fix incorrect type hints on getFullyCoveredSiblings() and
getCoveredSiblings(), luckily it was harmless
Change-Id: I8cc12450378efa7434c4d66882378b715edd4a70
`tagName` is only defined on Element, and it returns its tag name.
`nodeName` is defined on Node, and it returns the tag name for Elements,
and a string like '#text' or '#document-fragment' for other types.
We were using both, which made it harder to reason about what types
we're dealing with.
Change-Id: I8e621e5872bdf78c84ec553cfbfcdbf0192f0589
Added in 76289cdf73,
should no longer be needed since we switch to Parsoid's
HTML parser in 3e6ab2c4d2.
Change-Id: Ic0b7ed8089b71f2338e604f68d547759e069f0b2
Also special-case thumbnail wrappers generated by
MediaTransformOutput::linkWrap, for compatibility with
TimedMediaHandler.
Bug: T301427
Bug: T302296
Change-Id: I7f48d8b2261507c5a33526c54109f5187d062ed3
When we encounter a node that doesn't represent comment contents, e.g.:
* a [reply] link we inserted (T297034#7641334)
* an {{outdent}} template (see changed test case)
…we should ignore it together with its descendants (like in
Parser#nextInterestingLeafNode), instead of processing descendants
and possibly detecting comment contents in them.
Follow-up to 8de940b587,
72b9c2c6f5.
Bug: T297034
Change-Id: Ib2fa40c5fa389572b0e88ef558728fa06e3621b0
In PHP, use DOMCompat::getClassList(), provided by Parsoid.
In JS, use `.classList`, available in all supported browsers.
This may fix some bugs where we were incorrectly checking for exactly
one class. The change in isOurGeneratedNode() is needed for
Ib2fa40c5fa389572b0e88ef558728fa06e3621b0.
Change-Id: Ia28d31678fd3d617b69280c4b7857755300fa515
Reimplement getFullyCoveredSiblings() using compareRanges(), which
checks basically the same thing, but works better and I like it more.
Bug: T297034
Change-Id: I33dc1d088bdee984064315290e378bfbfa830b10
Previously: 569db3603c (2020-06).
Unfortunately we've found cases where the previous implementation
doesn't work correctly, resulting in comments being added to the wrong
pages or page corruption.
Bug: T289873
Bug: T298051
Change-Id: Id867b3005ebc46906d6df852a525fcaec9e6b19b
Usually this isn't a problem, because the comments are marked as
template-generated and we don't allow replying to them. But we had a
special case where we were trying to skip over some invisible
elements, which was causing us to skip into the middle of the
about-group in some cases. When Parsoid sees that, it serializes the
contents twice.
Bug: T290940
Change-Id: I9fe0b8d43ab874ccef371990799f77bfc46bc954
These types can be passed a parameters to any file without
creating a dependency, so it makes more sense to allow
the globally.
Change-Id: I5504465fd997b46547642e7046993b370b85586e
* Move getTitleFromUrl() from parser to utils. It's a generic method,
the PHP equivalent is already in utils.
Bug: T277371
Change-Id: Id960e5f60af02bdeb0a3a68f43b7a695eb035139
Problems with the current setup:
* Each CommentController must have exactly one link.
For T277371 we want multiple, and for T282205 we might want zero.
* CommentController objects must be constructed immediately.
They are implemented to make this pretty fast, but it's still
unnecessary work to do on page load.
* Only one link may be activated at a time, and activating one affects
the styling of others, so CommentController has to use global state
to check if it can set up and to update them.
Instead introduce ReplyLinksController, which knows about all reply
links and which one is active, and emits events that allow
CommentControllers to be constructed on demand.
Change-Id: Iabdeded2e71e598ae78703a6ff9410d0cfba397c
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
Now it detect signatures generated by en.wp's {{Undated}} template,
and signatures of people who do weird stuff to the timestamps.
Bug: T275938
Change-Id: I27b07f6786ca5433a3c02a5fe68e4716d41401bb
In some cases it would return the parent node, instead of the siblings
it should return.
It's a private method only called by getFullyCoveredSiblings(), and
that method had a bug that cancelled out this one, so everything
worked correctly. But I want to use it elsewhere now and ran into it.
Change-Id: Ic12f007d57a8502a1bea5f0af17b29e9d59093d6
* Ignore rendering-transparent nodes between discussion comments.
* Improve isRenderingTransparentNode() so that <link> nodes
representing TemplateStyles are not considered transparent,
otherwise this would undo ae920b831f.
Using a regexp from Parsoid.
Bug: T272746
Change-Id: I0b3c3251156ba6c4826abf5ba44ea93f80ebc01d
Add yet another tree walking utility: CommentUtils::linearWalk().
Unlike TreeWalker, it allows handling the beginnings and ends of nodes
separately – kind of like parsing a XML token stream, or kind of like
VisualEditor's linear model.
(Add unit tests for this utility. The simple.html test case is copied
from [VisualEditor/VisualEditor]/demos/ve/pages/simple.html.)
Use this utility to stop skipping when we reach either a closing or
opening block node tag. Previously we'd skip over such tags inside
nested "transparent" nodes (like <a>, <del>, or apparently <font>).
Bug: T271385
Change-Id: I201a942eb3a56335e84d94e150ec2c33f8b4f4e0
As a result of 0fc71f60cd, "empty" text
nodes (containing only whitespace) at the end of the comment may be
inside the comment's range, and trying to ignore them caused the
ranges not to match and the frame not to be detected.
Now the code works whether they're inside the comment's range or not.
Add a test case for wrapped discussion comments with HTML comments and
with whitespace.
Bug: T250126
Bug: T268407
Change-Id: I2217ff5a635fd1c9c9e803f46795b1bfb3d17535
While working on T270009, I noticed that <style> and <link> nodes
are treated differently, which seemed weird. Rewrite this again,
hopefully this is the last time.
The changed test cases also involve <area> and <input> nodes,
and the new results make more sense to me.
Bug: T264116
Change-Id: I3af90c84768a4b3dc53446927f4dba6f72175a2f
