For preview parsing, the span containers are empty.
Not sure why not strip the container along with all generated
contents, but this should be the easiest fix.
Bug: T321185
Change-Id: I9afb2d0f543b79dbac8a652236fe55284de542a8
The VisualEditorFeatureUsage instrument is a candidate for migration to
the Metrics Platform [0]. The first step of the migration is to log
events both using the Event Platform directly (i.e. via
mw.eventLog.submit()) and using the Metrics Platform client (i.e. via
mw.eventLog.dispatch()).
The Metrics Platform Client can mix in additional information -
so-called context attributes [1] - based on the stream configuration.
The majority of the default values mixed into each event via the
mw.eventLog.Schema defaults mechanism are already known to the Metrics
Platform Client.
Note well that the Metrics Platform client will not log an event without
one or more streams being configured to receive that event. Therefore,
this change is a NOP.
An example stream configuration is given in [2].
[0] https://wikitech.wikimedia.org/wiki/Metrics_Platform
[1] https://gerrit.wikimedia.org/g/mediawiki/libs/metrics-platform/+/aed6738b845/js/src/StreamConfig.d.ts#31
[2] https://phabricator.wikimedia.org/T309602#7973206
Bug: T309602
Change-Id: Ib919ae0e3f404c85cef17637ea91bb95d5030cf1
... for the sampling rate for the VisualEditorFeatureUse schema.
Bug: T312016
Depends-On: I259757db0c4441a3fcfce505d5bc82dcf2acf58c
Change-Id: I1c7f9c06384549deac2787f5df93c0078b6402af
Pages can be empty (blanked) without outputting the noarticletext
class, so just suppress the top border if the banner is the first
child of the document.
Change-Id: I808160a7f73a9a976d25e77f4bd47727a57b70c0
If the widget has just been opened, it should already be in view,
but when we are recovering from auto-save it might not be.
Scrolling it into view lets the user know sooner that a draft is
about to be recovered.
Change-Id: I2b8232edc20e71b04a3f106107c0c7bc6333f66a
Also ensure that when we click the button, we scroll the widget fully
into view below the sticky header.
Bug: T318474
Change-Id: I394f02912cd6ab2773552a7364691ef89a17369c
Computing the name, for headings, requires determining the oldest
comment. This must not be done before all the replies are added to the
thread.
Follow-up to e24550fae9. It was
already *technically* incorrect before then because it was generating
the name based on the first comment in the thread, but that was only
not the oldest in very unusual cases so it was fine. That commit caused
the thread summary to be cached when first requested, however, which
made future requests for oldest/newest comment and authors to be
incorrect.
Bug: T318057
Change-Id: If0bd6caf88e72cd3f91e7f0633c40b445f5e2246
* Make "X comments" appear on a new line deliberately
* Remove parentheses around "X comments"
Bug: T309463
Change-Id: I803eee9db59c633f129f15e436242a12bcc627f0
When using ReplyWidgetVisual, this would just crashes in #getValue.
When using ReplyWidgetPlain, if the timing is right, this might update
our auto-save after the value was cleared in the teardown code,
breaking the way we restore the reply after showing new comments.
Bug: T316074
Change-Id: I23c5f17a6ff9a6ec9c73a176e4cc60e3ad96e7f1
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
We already do this while editing so that users can quickly see which
tabs they are actively working on.
This extends the functionality to the reply and new topic tools.
Bug: T262066
Change-Id: Iae662ad26072617aad71e519bb6c3cbb19ef1246
The subtitle prop and some others were missing from the action=parse
request. Use the same props as the VE edit API.
Change-Id: I8e9cc735d3ee50dfe0fbe0349713d88654ad9fd1
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
The bottom margin used to collapse into the next heading, but
MobileFrontend changed how section hiding was done.
Change-Id: Ide10e9f17ad38d672958e2c3a43c2eb2cfdd82ae
Relying on :target getting set means we can't use
history.pushState to change the URL without scrolling.
Should conform to https://url.spec.whatwg.org/#percent-decode
Change-Id: I4ccc3fd745884849a781a9f7fc8b00b8b689e20a
Follow-up to 31c57d594a. This was
causing an exception, preventing the page contents from updating after
saving changes.
Change-Id: I8c9ab51385172056be9032ec0087f64ff34b6709
It should appear below any page content, at full width.
Currently it gets squished by floats on some pages.
Change-Id: I4c4107d438dfd06eec21badce5f216aa2c137272
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
They were only disabled in the reply tool, because their wikitext
markup doesn't work when indented. This is not a concern in the new
topic tool.
Rename our sequences that display the wikitext warning to avoid these
items showing up in the command help dialog, and remove overrides for
global ve.ui.commandHelpRegistry. (As a result, the dialog now lists
the blockquote command, which was previously missing even though it
was enabled, but only with the 'Ctrl+8' shortcut and not 'Type ":"'.)
Also remove filtering rules for external paste.
Bug: T311653
Change-Id: I4557c376fcf099d81e91ae5c2b18301d97921180
Since writing this feature we added the ability to update
the page dynamically on mobile, so this shoud just work now.
Bug: T301929
Change-Id: Iceb5d8c90a892807745ba7133b7dd389293d3327
In general we are hiding non-content elements in the preview
and this matches the behvaviour of the article editor.
Change-Id: I6669d8cc737214d464b662ccc6900ccce229b3d7
:first-of-type selectors refer to the node name, so just
use :first-child and :last-child for the open and close brackets.
Change-Id: I2e8ca7e99d1262522ed2d3b5133093817af8ad29
If some content is loaded without re-initializing DiscussionTools
(this currently happens on live preview), then a link to a comment is
clicked, we will not find the comment, which previously caused an
exception. Just ignore this, other code paths in highlighter already
handled this situation in this way.
Bug: T309423
Change-Id: Ie46fd782b8a45907dc6fc604691575c4b55d08ce
Unlike PHP DateTime, and unlike the .toISOString() method,
the Moment.js .format() method will output localized digits.
Bug: T312828
Change-Id: I49a250b1ad74d9395bf3f994379613cc121b5be4
When the input to moment() doesn't define a timezone, it
defaults to the local timezone (as defined on the device,
not in user preferences).
Bug: T312828
Change-Id: I8d2e1c2f6feb5584af4bbf9fef39567eb87780ff
We remove the [reply] and [subscribe] links when they should not be
visible (controlled by 'enableSectionEditLinks' option, which is
disabled when previewing).
Bug: T309423
Change-Id: Ie0d3fba2c4d166daac3ea2e117a246c9584284ca
Use 12.8px for interface, and 12.7px for content.
Move mode tabs away from the reply body so the underline can be seen.
Change-Id: I9cb9a611ca64595152f800e1e15b640402f6cb74
Previously this was reverted in I28a5c774f5f after the initial
commit I3736b488046 caused T312177.
Bug: T311760
Change-Id: Iba65615378b183f0c970afff565f45cc878f1d71
Vertical space is at a premium on mobile with the virtual keyboard.
The widget will auto-size when more text is entered.
Bug: T311836
Change-Id: I03ff7217f9ffe217f93a3dc95db2df018bd0d221
We tried to align the buttons with the far edge of the page, but due
to skin styles setting `overflow: hidden` on the content, this is not
possible without causing the focus outline to be cut off.
Bug: T311662
Change-Id: Ie7c10c7564450c06231632892341c7fb232e7041
The icon element should be aligned with the right side
of the content area, like the expand/collapse button, and
the edit pencil in the main namespace.
Change-Id: I191a5984bafc6a478674dd5dccff0594a743ff18
* Use sibling selector instead of negative margin to reduce
space between hint and reply widget.
* Remove upstreamed right-padding for close button.
Change-Id: Ie51cd2208c2bebcbfeff08c249389085523193f0
For comments posted on or after 2022-07-12 (configurable), use a
shorter format, identical to the timestamps MediaWiki uses in URLs.
Changing the format for already existing comments would involve
difficult migrations, therefore we elect not to do it.
Bug: T304595
Change-Id: I387051a6a3a1d84cfae45c3e1516db870cc8b977
We cleared the highlight outside the setTimeout(), then set a new one
inside the setTimeout(). If this method was called twice quickly, two
highlights would be created, and the first one of them would become
impossible to clear.
Move the setTimeout() outside of the method to avoid this.
Bug: T311021
Change-Id: Ic8b2cf2a782a429c4ea073871efd215f4b300ed8
* Fix math and styling for RTL
* Make highlights for headings and framed comments cover them fully
* Align the far edges of wide comments
Bug: T309444
Change-Id: Iadbf83e02801fd9bd6336c84015994ee2819ce51
It's needed for the fix for T303396 to work correctly (in the case
where there's exactly 1 thread item on the page), and for the
highlight improvements in Iadbf83e02801fd9bd6336c84015994ee2819ce51.
Change-Id: I2b2f5535ebbf792910133622a271ac1491c74de6
Rather than making our own ThreadItem objects from scratch,
reuse those used by the rest of the code.
Change-Id: I67f5ea3ad26baffe51b66d0c75467ac00185dd0a
We switched to our memory-wrapped store everywhere outside the
VE surface, but not in the VE surface itself.
Change the way we construct MemoryStorage objects so that they can
be constructed from a specific mw.SafeStorage instance.
Ensure MemoryStorage's cache is populated with initial storage object
data, and that the cache is copied over when we create derivative
objectes.
Bug: T310438
Change-Id: I652428f114dc5169195887cb8ca719ae196bb76f
Since change I29f6af4cc7c71a63a6d1bafc53d16b9abd1b60ec, VisualEditor
handles the Escape key to emit a 'cancel' event. We need to listen to
this event to allow Escape to close our editor when the surface is
focussed, in addition to the existing handling for the Escape key.
Change-Id: I8c0f2678d104a76d6a223aca9b57123b9f587ac7
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
The parent constructor may trigger read/writes in the future
so this.data should always exist as early as possible.
Change-Id: I79a795a26c4ce1381d2a0b15329f7df1d6ab77ca
The EditAttemptStep instrument is a candidate for migration to the
Metrics Platform [0]. The first step of the migration is to log events
both using the Event Platform (i.e. via mw.eventLog.submit()) and using
the Metrics Platform Client (i.e. via mw.eventLog.dispatch()).
The Metrics Platform Client can mix in additional information -
so-called context attribute [1] - based on the stream configuration. The
majority of the default values mixed into each event via the
mw.eventLog.Schema defaults mechanism are already known to the Metrics
Platform Client.
Note well that the Metrics Platform Client will not log an event without
one or more streams being configured to receive that event. Therefore,
this change is a NOP.
An example stream configuration is given in [2].
[0] https://wikitech.wikimedia.org/wiki/Metrics_Platform
[1] https://gerrit.wikimedia.org/g/mediawiki/libs/metrics-platform/+/aed6738b845/js/src/StreamConfig.d.ts#31
[2] https://phabricator.wikimedia.org/T309013#7953227
Bug: T309013
Change-Id: Ic88225111132b440657aab85c34b36a642a7e602
When moving the edit mode switcher outside of the toolbar actions,
we apparently forgot that we can disable this feature entirely.
Also remove $overlay: true, which isn't needed after the changes
from T307849.
Change-Id: I005cf18853305edd162c613f60b1ac45f42c3093
Topic subscription test is going to be all logged in users only, no
transitory enrollment conditions, so we can remove the anonymous user
handling and DB writes.
Bug: T302515
Bug: T304030
Change-Id: I5e57bb9b7958576f3a04373748331a86f4626fb5
We previously ignored them before timezone indicator (e9c401e3aa),
but they can end up in other places too, e.g. after the time.
Now we ignore them after every token. This is way overkill, but it
shouldn't hurt.
Bug: T308448
Change-Id: I20f7aaa34dba23f2a2faf1be258c1aea32ab770f
This can be merged a few weeks after the commit that introduces
it (If28e1588742), once all the HTML caches have been purged.
Change-Id: I96d2de081a23137fe11a66c8213535219ebfb230
If the user can't edit, tell them why, instead of trying to
advise them to use the edit button that they cannot see.
Bug: T303110
Change-Id: Ibc41732a55443331e432987e630976610901c599
getPageData() had an optimization where, when it was called with
isNewTopic=true, it would skip querying 'linterrors' and
'transcludedfrom' data and return fake empty responses.
The issue is that they were cached, and then returned when it was
called with isNewTopic=false later.
Remove this optimization, as we make those queries anyway on init.
Change-Id: I1990b16f574c1aec385bd0fa46a9f39c6c2c1df2
mw.util.getUrl() already encodes the values, encodeURIComponent()
isn't needed.
Also, slice off the leading '?' – while MediaWiki seems to accept it,
it looks like it is never present in any other place using this
parameter (this is fine even if there are no query parameters).
Bug: T308198
Change-Id: Iee3747ab53e3b0a5a0b43a7701205ac0c7f07e7f
Previous attempt: I40da5272fd9c44a5a81e303349d0e8fc404e344d.
Limit the hack to only end markers and only inside headings
to avoid causing issues with page content.
Bug: T298371
Change-Id: I9dc95e17752b7f048eb3343dbeea3526359197c3
The old section=new wikitext editor does this, and I'd rather support
the same behavior on desktop and mobile than different ones.
Follow-up to 89a1efc9d8.
Change-Id: I5371f932680638ee159f2f713bbca44bba5675aa
jQuery#add sorts the nodes in DOM order (and removes duplicates).
We did not sort the ranges, so they could end up in a different
order, causing #update to apply the CSS to the wrong nodes.
This was usually harmless, but it could cause incorrect rendering
when some elements are positioned or hidden.
Change-Id: Ic359db08dafa3d86a5778426a7dc2419c94feabd
Set the location hash to that of the new topic, then wait
for the section to expand before drawing the highlight.
Bug: T306399
Bug: T301840
Change-Id: I8bc78b49f359e10279584c16269fcb78a087eae0
Cases where saveSuccess wasn't logged:
* creating a new page with the New Topic tool
* any replies on mobile
* successful replies made through transclusions which then couldn't
purge the current page
These were all cases where we abandoned the post-save process early to
reload the page.
Bug: T305541
Change-Id: I1366a3e0a4b03ac67f926284f1aa718ae552d852
Also fix a bug where headings would be ignored while checking for
comment frames. See task for detailed explanation.
Bug: T303396
Change-Id: I6495826b4b050ea80680e0798ac6ab4497a7c09e
The CommentController stores a list of the new comments that
triggered the reload warning. Just highlight all the comments
in this list, as we always reload to the revision we got from
the same API call.
Bug: T303261
Change-Id: I960f3cf33e25004bad6ab6df907121a87555aaf4
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
We call CommentUtils::getHeadlineNodeAndOffset() before constructing
the HeadingItem in CommentParser, so the range's startContainer
is always the headline node.
Change-Id: I2afb6ba9100e785cd91f31d82f4cea59fa8b5443
`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
Raise scope of pageThreads variable (renamed from `result`)
so we don't have to re-bind when it is updated, and use a
flag to ensure we only bind the handlers once.
Bug: T302810
Change-Id: I0a5b122b8e3f656f28f3dd4fa6aca8e7839be06b
Links are generated to:
* Single comments, by a user or by a notification
* Potentially multiple comments (new comments since T),
usually just by notifications.
If in either case no comments are found to highlight, warn the
user so they get some visual feedback that the notification worked
as expected, but that the comment was just deleted since.
Bug: T301602
Change-Id: I090c904481cf6f3e9f964fa43b10001e75b0bc90
Also special-case thumbnail wrappers generated by
MediaTransformOutput::linkWrap, for compatibility with
TimedMediaHandler.
Bug: T301427
Bug: T302296
Change-Id: I7f48d8b2261507c5a33526c54109f5187d062ed3
We were rendering the preview in a completely different way from how
we would add the real reply, and the results would be different
sometimes, particularly for multi-line comments with messed-up markup.
Render it server-side instead, in a very similar way to real replies
(generating a DOM list node and transforming it through Parsoid),
although without the whole context of the page to improve performance.
We can remove a lot of client-side code that was used solely for this.
This will allow the preview to accurately display the signatures when
we change how they are added (T278442), without us having to implement
those changes again from scratch for the preview.
Change-Id: I53341f4d4075c25b67ec3b3032bff9b8a880dcd3
Goal:
-----
Finishing the work from Iadb7757debe000025e52770ca51ebcf24ca8ee66
by changing CommentParser::parse() to return a data object, instead of
the whole parser.
Changes:
--------
ThreadItemSet.php:
ThreadItemSet.js:
* New data class to access the results of parsing a discussion. Most
methods and properties are moved from CommentParser with no changes.
CommentParser.php:
Parser.js:
* parse() returns a new ThreadItemSet.
* Remove methods moved to ThreadItemSet.
* Placeholder headings are generated slightly differently, as we process
things in a different order.
* Grouping threads and computing IDs/names is no longer lazy. We always
needed IDs/names anyway.
* computeId() explicitly uses a ThreadItemSet to check the existing IDs
when de-duplicating.
controller.js:
* Move the code for turning some nodes annotated by CommentFormatter
into a ThreadItemSet (previously a Parser) from controller#init to
ThreadItemSet.static.newFromAnnotatedNodes, and rewrite it to handle
assigning parents/replies and recalculating legacy IDs more nicely.
* mw.dt.pageThreads is now a ThreadItemSet.
Change-Id: I49bfe019aa460651447fd383f73eafa9d7180a92
Goal:
-----
To have a method like CommentParser::parse(), which just takes a node
to parse and a title and returns plain data, so that we don't need to
keep track of the config to construct a CommentParser object (the
required config like content language is provided by services) and
we don't need to keep that object around after parsing.
Changes:
--------
CommentParser.php:
* …is now a service. Constructor only takes services as arguments.
The node and title are passed to a new parse() method.
* parse() should return plain data, but I split this part to a separate
patch for ease of review: I49bfe019aa460651447fd383f73eafa9d7180a92.
* CommentParser still cheats and accesses global state in a few places,
e.g. calling Title::makeTitleSafe or CommentUtils::getTitleFromUrl,
so we can't turn its tests into true unit tests. This work is left
for future commits.
LanguageData.php:
* …is now a service, instead of a static class.
Parser.js:
* …is not a real service, but it's changed to behave in a similar way.
Constructor takes only the required config as argument,
and node and title are instead passed to a new parse() method.
CommentParserTest.php:
parser.test.js:
* Can be simplified, now that we don't need a useless node and title
to test internal methods that don't use them.
testUtils.js:
* Can be simplified, now that we don't need to override internal
ResourceLoader stuff just to change the parser config.
Change-Id: Iadb7757debe000025e52770ca51ebcf24ca8ee66
The Data class contained utilities for two unrelated purposes.
Split each half to a separate class.
Notably, this improves the signature of the getLocalData() function.
Change-Id: Icde615fb9d483fee1f352c34909b37f8ffde8081
(suggested by PhpStorm)
composer.json:
* Document required PHP extensions
Parser.js:
* Remove incorrect param documentation
* Fix some typos in comments (missing parentheses)
CommentParser.php:
* Fix some typos in comments (missing parentheses)
ImmutableRange.php:
* Remove unused property
* Add a `throw` to indicate that code path is unreachable
SubscribedNewCommentPresentationModel.php:
* Add missing `return false`
CommentParserTest.php:
* Remove unnecessary pass-by-reference
CommentModifierTest.php:
* Remove unused variable
CommentParserTest.php:
* Don't construct Element objects directly. PHP's DOMElement allows
it, but Parsoid/Dodo's doesn't, and we use the latter for static
analysis. This generates all kinds of confusing warnings.
Change-Id: Ia9598ebea0e99830dd485296e94a9d96acc4b258
