The .ve-init-mw-target-surface class is no longer added since
Ic5320f6747907542285674d386c7a59c9e857f0a.
Also apply changes similar to I8d20a830dc48f6a098b0f9e9a7c7c1656de0fe56
to avoid potential styling issues with nested surfaces.
Bug: T284567
Change-Id: I58a49f0137e8804fbd73de20048eb2ffdbbfbe77
Previously, our highlights were placed in a node at the end of
the page, and positioned absolutely in relation to the whole
page. Now we insert the highlight in the DOM near the comment,
and position it in relation to that.
This way it remains positioned correctly when the page shifts
(e.g. collapsing the table of contents), and disappears when
the page content is hidden (e.g. opening visual editor).
Bug: T281471
Change-Id: I60afc4b94b2e23376105638542563e595a1811d9
The modifier crashes if endContainer is a <p>/<dd>/<li> node, and
it often is if we try to exclude the end marker from the range.
It doesn't matter for T281471, so let's just leave it that way.
(This is also more similar to the range produced by our parser.)
Follow-up to c4ba8e921a. No idea how
I missed this when testing that patch.
Also, improve comments.
Bug: T281471
Change-Id: If6aba34acf29c37d06fb0ca92547f78b58695597
This produces nicer results when we call Range#getBoundingClientRect
on them later, when drawing comment highlights (using Firefox).
I'm not sure how this will affect everything in modifier, but it
doesn't seem to be causing issues. If it does cause trouble, then we
should instead adjust the range this way in controller#highlight.
Bug: T281471
Change-Id: I6f204b858990023f42f564cca7a2d24d322f872e
Per Manuel Arostegui in T263817#7033384. The limit is 5000.
(I picked it arbitrarily, there's no real rationale for it.)
Also log a warning when any user reaches half of the limit,
so that we might make a decision about changing this mechanism
before it starts affecting users. Maybe at that time we'll
have data to show that it's safe to remove the limit.
Bug: T263817
Change-Id: I18a8ee0ad7383759229c5721d5253fb591457d4d
The issue occurred when replying to a comment consisting of multiple
list items, starting with a <dt> (instead of the expected <dd>), so
that the comment is considered to be unindented.
Modifier tried to add the reply directly inside the list (<dl>) rather
than inside the last list item (<dt>), which caused it to be confused
about indentation levels and try to un-indent more times than there
were indentations.
The simplest solution, given the existing code, is to add the reply
outside the list instead, in a new list. This results in a "list gap"
(<dl><dt>...</dt><dd>...</dd></dl><dl><dd>...</dd></dl>), but I think
it's acceptable for this rare case.
There are separate tests cases for old Parser and for Parsoid HTML,
because they parse the original wikitext differently (with the old
Parser producing HTML with a list gap too).
Bug: T279445
Change-Id: Ie0ee960e7090cf051ee547b480c980e9530eda51
If curLevel or desiredLevel are calculated incorrectly, this loop
could never end.
In JS, something would throw an exception before going infinite, but
PHP is happy to trot along despite accessing properties of null and
attaching multiple children to a document node.
Bug: T279445
Change-Id: I1784f550ec3a23dcded4f2b1def97e51cb414b7b
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
As of 7ad6328223, we also use this data
to check whether comments exist on the page, not only whether they're
transcluded.
Follow-up to 42ce942c86.
Bug: T275821
Bug: T273413
Change-Id: I95eb85354e7b84cc10ab703d28315d0667696f4c
The information is already included in the VisualEditor metadata request.
Bug: T276393
Bug: T270803
Change-Id: I45a232dcd23418da0711834bcc369a9a718006b0
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
Topic title field and old wikitext reply field had only placeholders,
but no labels.
Mode selector had labels on individual items, but not on the main
control.
Change-Id: I422e7e5baa8711340a1bb82255e788f2272c45c9
When there are just two modes, using arrow keys to switch between
them is not intuitive. The focus moving from the selector to the
body widget afterwards is even less intuitive.
Override default TabOptionWidget to allow options to be highlightable
(not just immediately selectable), and mark the current mode's tab as
disabled instead of selected (but make it look selected).
This results in intuitive keyboard interactions (tabbing to the widget
highlights the other tab rather than the current one, pressing enter
switches to it).
Bug: T274423
Change-Id: I9d358d5f301cbf081380ef5d34ccc8c4e146652e
In some situations we didn't pass the mode in the config.
Simplify some redundant code that got messed up when we
introduced the NWE source mode.
Change-Id: Ia838fc6752d411f70c8cc6a36d84d2a851fd68bd
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
When posting a new topic, if your comment ends with a template
that has trailing line breaks, the signature will be added as a
preformatted text (which is arguably a bug: T255741).
Preview, however, did not reflect this bug, because of the <span>
markup added to fade out the automatic signature.
Change-Id: I062b01a035e22edfca752a49c5e2433b3f7fb4f6
We have logic above to exclude mentioning yourself, but this
is overridden if you are commenting on your own talk page.
Change-Id: I2858c79bd9f1cb733f105825e17f9df75859e40a
There are no available tools that use this, but it
can be tested using:
ve.init.target.surface.executeCommand('specialCharacter')
Change-Id: I853e6a3b9bd3caff018b6fe22cea9b1c6a428dff
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
The horrendous 11-line if() condition did not correctly handle
signatures wrapped in inline formatting markup, like <small>.
Instead, implement this logic in the code for skipping to the end
of a paragraph, which didn't exist yet when that condition was
added, but seems like a much better place to check this now.
Bug: T275934
Change-Id: I5cccff889b5e15b5f8fde0538bf4bccb22e762cf
Same pattern as is now used in VisualEditor. Generate an initial session
ID when the file loads, so if any VisualEditorFeatureUse events trigger
before the init there'll be a session ID for them to use.
Bug: T275051
Change-Id: I4f25e9e1e195c11129044868eb67fcc2f4494ffd
Yes, this is still needed, removing it causes failures in tests
(and the old outputs look better).
Change-Id: I5bcedb0295a1f0ac4f6e51eaa9a9e072d8236f3c
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
Since it's no longer tied to the "Reply" button, we can move the code
from ReplyWidget to NewTopicController, where it belongs.
Bug: T272543
Change-Id: I8c90ffb772573d22d26c608d45877ee948fd232d
* 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
Several of the messages talk about clicking the reply links, which
seems unhelpful. Also, showing a fullscreen popup on page load seems
very unhelpful too. (Itoldyouso that we shouldn't use popups.)
Bug: T268069
Change-Id: Id1312cf06200fb45a28b39481a99cc2c96603fa4
If the user is clicking on a new topic link, and a reply widget is
open, attempt to close it instead of doing nothing.
Bug: T272545
Change-Id: I1903f5ae4c9e98c4b3a4703ad0e44d772894592a
We'd like the [reply] links to behave differently if other
CommentControllers are already active, but each CommentController
doesn't know anything about others; only the main controller.js does.
Change-Id: Ic21b2d40d213a325509822f703709f52aa8dc8d7
If the module is loaded on a page where DiscussionTools is
not supposed to be enabled, wgDiscussionToolsFeaturesEnabled
will be undefined, and the code will crash before it can set
the cookie that enables it for future page views.
Bug: T272850
Change-Id: Ia1c40cfc3cbee62823f1806bd20229883905677a
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
Remove unused error codes, update the ones that changed, and sort in
the order in which the types are listed on the schema page.
Bug: T272162
Change-Id: Ib16f7e11ff81d43443b08cf704ce5196420671e4
Don't assume a feature is available because the code has
loaded and the user option is set. Export the logic from
Hooks.php to the client.
Change-Id: Ica0e58de7ed0d59e3b09645193eb2b691ae41c39
If DiscussionToolsABTest is enabled (set to `all` or a feature), logged
in users who have never used the tool before will be assigned to an a/b
test bucket. If they're in the test bucket, they get the feature
enabled.
If they manually set their beta feature preference, we don't override
that but do maintain their bucket for logging purposes.
Bug: T268191
Change-Id: I9c4d60e9f9aaef11afa7f8661b9c49130dde3ffa
