The original intend of that rule was not clear to me and it seems
it got dragged along for some time. I checked the DOM in the
collapsed state to see where this could even apply in the new
interface and found no other place than the one where we overwrite
it again.
I guess it can go away.
Change-Id: I2886db33c5f06d1c49acc4743c9acc198339de36
These are internal to the two-pane layout and sidebar, so the dialog
doesn't need to be involved.
Bug: T310866
Change-Id: I5f05bc119dc213d8e31db62a3808a2fadaf35d99
This is a direct follow-up to I7f22e4b. I found that the way the
margins have been arranged became a little more complicated with
I7f22e4b. This patch tries to go back to the – I think – simpler way
it was done before.
It should look the same as before down to the pixel.
Bug: T311223
Change-Id: I2c7789922078fa98f15f0b65de4c0efdf878a13a
The one usage of this wasn't working. FIXME: maybe we want to
restore the behavior, to focus the first parameter in the dialog
after opening.
Bug: T310866
Change-Id: Ibe0151fbedb3a9716bb231b5d398c4ae670fd667
This defaults to false. It looks like this default is used in the
MWTemplateDialog base class. However, it also looks like this base
class is never used on it's own. All users we know use the subclass
MWTransclusionDialog where "editable" is true.
https://codesearch.wmcloud.org/search/?q=%2C%5Cs*ve%5C.ui%5C.MWT(emplate%7Cransclusion)Dialog&files=%5C.js%24
In the process I also remove some comments that literally repeat the
code and don't add any knowledge because of this.
Bug: T310867
Change-Id: Ie245aab80d1e77a8406f5591062e9cf49fd9613f
This code was meant to select the first parameter of a template the
moment it is added. It's at least partly broken because it doesn't
consider the so called "prompted" parameters that have been added
just a few lines above. It's a questionable feature anyway. We are
going to refine all focus-related behavior anyway. Let's remove this
broken stuff and reimplement it later (probably in a different place)
when we continue working on this.
The FIXME was added in I720ce1a.
Bug: T311223
Change-Id: I1801efe38387b5e7a1b76417c1e5d7db4e4b96d0
This was done in 3 rather "random" places:
1. Whenever a template is manually added. But rather late, after the
template was added, in an event handler that is about focus
behavior. It should not continue to manipulate the template that
was just added.
2. When the dialog opens with a template preloaded by name, as it is
done from the citation menu.
3. When the dialog is about to finish loading.
This patch fixes 2 issues:
* Get rid of a duplicate call (number 2 and 3) when using the
citation menu.
* Move number 1 to a place where it's executed much earlier, and
only when the user clicks "add template" in a template placeholder.
There is no other way to add a template to an existing transclusion,
but it's still a more appropriate place I feel.
Bug: T311069
Change-Id: I8a65ad703b95ba2092e9ef73493e9903e96b0dd6
This still works for a dialog with just the template placeholder,
because the page is already chosen as "current" in the stackLayout.
Bug: T310866
Change-Id: Ibc1d33b02d34f70548d9f7365e085847ef0b2a51
Includes moving CSS that already moved from the TemplateDialog CSS
file to the DialogLayout LESS file.
Width and height had no effect. Neither on desktop/mobile. I guess
the control's height covers for that. Float could be removed due to
the flex layout.
Some more specific rules could cover for the !important overrides.
Change-Id: I7f22e4be37c8f227845aed97281faefe26241091
Pre-parsing with $.parseHTML is not required as we
1) no longer modified the DOM before appending
2) trust the HTML coming from the API
Change-Id: If549a0e647ce830d4f5de2bb94c08a895e460667
This causes one small behavior change: when deleting one of several
wikitext transclusions or a part following a template without
parameters, the selection would previously be set to the *previous*
part, and now it's the *next* part. This is arguably more consistent
with the behavior when removing eg. templates with parameters, after
which the next part is selected.
Doesn't affect removing parameters.
TODO: We might want to restore the already-broken behavior to focus
the first parameter of a newly-inserted template.
Bug: T310866
Change-Id: Ic5f47e31512d1a3949caf60613bd05b9a3bdf478
This place is much better in so far that it much more obviously gives
us the guarantee that the sidebar and the content pane are always in
sync.
Bug: T311069
Change-Id: I01a7914fcba5d573abb957d0e34fa895874bd94e
According to my test one of the two places where this property was
used is impossible to reach with the property set to false. This is
the one I remove in this patch.
This also removes a related method that is entirely unused.
Bug: T310867
Change-Id: I9579a5d894d60560cec77dad7f1e796f8dfca06f
Includes adding a less file for minerva layout rules. To make best
use of the less shortcuts and avoid duplication, some selectors
have been slightly changed. Outcome should still be identical.
Change-Id: I92179ecf6045c938cace0e7e809b7ad4cf035727
Follow-up to Idae6755c90eacaab1a9daa88c6e28850d427810c. These styles
still used #content instead of the new way of specifying the target
container.
Extra cleanup:
* Don't apply custom styles to #firstHeading when it's outside of
the target container
* Remove unnecessary rule (it was added in 6d8fbd8221 to support
an absolutely-positioned loading toolbar, but it is no longer
positioned that way)
Bug: T310839
Depends-On: Idb2a743c93316786d6d36e1989cf6620a6092281
Change-Id: Ib5e8f98eacf763e360fae79337d3f2733b1a101b
In some cases ve-ui-mwTransclusionDialog-newSidebar could just removed
when there are specific enough classes present so that we can be
sure to not overwrite rules outside our use case.
e.g with the new ve-ui-mwTwoPaneTransclusionDialogLayout-stackLayout
Note that this will also remove some right padding on descriptions
where it seems that it was applied for no reason.
Bug: T310859
Change-Id: I6612fe1cb2ea6bd75e9bc3e6f4d6d8d0c4addc63
This data attribute, used to give skins the ability to position VE on the
page, should be prefixed with `data-mw` to prevent it being inserted on
the page by user generated content.
Bug: T310197
Change-Id: Ia6f87535f11ccc7aadb26b7dd9e1ac8a867c377c
Leaves a reference in the TemplateDialog so that we can slowly
migrate the many usages.
Makes no other changes to how the sidebar is managed (yet!).
Bug: T311069
Change-Id: I45403cd32e3adbe357ebad7bbc851f60d92751e5
The only event in use is "set". The others are never used. Which makes
sense. The driver for everything related to adding or removing parts
is the model. This widget is at the receiving end.
Bug: T310867
Change-Id: I4ef7e59ff05cb02aca59b76fdffa4f1fced76e33
It's never used in a different way. A lot of other code depends
heavily on this fact. Let's hard-code it.
Bug: T310859
Change-Id: I65bbaea47341d74b49ce447c896eb2340f730e17
The method in the model (where it belongs) was added via Iaf28035 in
2016. The other via I073c585 in 2014.
Bug: T310859
Change-Id: Idea322aec175600e3055a859ca987afc1fe6dd8c
Defines an HTML attribute, `data-ve-target-container` that gives skins
the ability to choose which element they want to act as VE's target.
This attribute addresses a need in the Vector 2022 skin, where the
default selector, `#content` was no longer suitable to act as VE's
content container and more flexible approach was needed, so that the
skin itself could define which element VE should use.
This selector falls back to `#content` as was the case previously.
Additional change:
Update modules/ve-mw/preinit/styles/ve.init.mw.DesktopArticleTarget.init-vector.less
to account for the planned change to retain line between tabs and
toolbar in new layout.
Bug: T310197
Change-Id: Idae6755c90eacaab1a9daa88c6e28850d427810c
This hasn't been necessary since ooui-js Id9cf086771, it was to prevent the
BookletLayout from scrolling to the top when removing the currently
selected item.
Reverts I0c1fddfa32b76621a9f1328c8173f0158386aee8
Change-Id: I5752fb273d0e2a3f0e7b6a044e69d68dc3e4b657
This was added via I3b792ff. It's about the old sidebar which isn't
accessible any more.
Bug: T311069
Change-Id: I29919285255a84bd58aa06ee1b2816d25a8112a6
This class was (accidentally?) removed in 2014 so the styles had
no effect since then. I decided on removing the code that was
unused for 8years. - Alternativly the class could be re-added but
I'm not sure if it makes sense.
See I4b2ba31bed5c4f80940623702d635cacd19e0a66 where this got lost.
Change-Id: Ifcbfc4273e41c08431c6f5b0ca2f2b6d25c34edd
Bring over the ES and Less implementation from ooui-js and rename to
fit it into the VE hierarchy, but don't make any other substantial
changes.
Inlines the wikimediaui-themed styles, ignoring apex styles, and
hardcodes the Less constants.
Bug: T310865
Change-Id: Id43dafdf11c5df0d7d78112e5f62a8599bdbc879
Technically the old BookletLayout sidebar is still there. But it's
never visible, effectively dead, and meant to be removed. This patch
just empties the OutlineItems of all template dialog related pages
without actually disabling the old sidebar.
Note this partly reverts Ie57f462.
Bug: T310859
Bug: T310866
Change-Id: Ic0b7d703f369045ed342426563f8eeb3e47046db
Value changes are triggerd and tracked from the parameter page and
change events will be forwared to the outline, if they are relevant.
Initial value is read from the model.
Bug: T308730
Change-Id: I0a3b0faf40aee44889404dcce31d850714360580
There are 3 ways to enter the dialog:
* Editing an existing template.
* Start empty.
* Start with a known template name.
The back button should only ever appear in one of the three modes.
The last mode is the one that's used in the Cite dialog.
Bug: T310602
Change-Id: Id23d3ac5e1715387c78916adeb8ca5f675005a5c
The outline item is only used by the new sidebar for the move and
remove flags it seems.
Bug: T310859
Change-Id: Ia74e5b0e3dbf81e745137b181ed34a4d48dac42c
Parameters do not have an "OutlineItem" anymore so the code for
that was completely removed.
Bug: T310859
Change-Id: Ie57f462b8fda4505b99ee5bc9d788908d18d9c64
Removing a class name that's going to be deleted from places where
it's not strictly needed, e.g. comments.
Bug: T307188
Change-Id: Ifb14695d05510d2c0e25623afa99c4e84af3aaf9
The message 'visualeditor-error-invalidresponse' doesn't exist,
it has been removed in 5f1c68945d.
MediaWiki API methods can return an error message for this case.
The message 'visualeditor-saveerror' is not used anywhere else.
Change-Id: I3f5617b94135fa602b714aafc0eb6b16f2cd77df
Instead of just suppressNotifications, also include:
* Document ID
* Storage interface
Depends-On: Id7ca1dcf84a82b8e16109659b8f0f0d9a5d064fb
Change-Id: I6ab00c089c9ae1a8bb05ce9405f1f1f2fd0915ca
The API error messages already explain everything well enough,
we pass them directly into a very similar popup in #loadFail.
This message is older than the pretty API error responses,
and didn't work with them well.
Bug: T306763
Change-Id: Ie0d8dc24c967cce02579d6c0539a55ba14372f84
Since VisualEditor cannot be sure about how skins have marked up the
page, the code that marks content as uneditable (not clickable) can
be tightened further to make sure the edit and view tabs are not
marked.
This fixes the issue where the tabs are part of the content area
Bug: T162503
Bug: T310197
Change-Id: I4e7388c2e3b681b766068639ab9325bb6289562d
As this is a surface command, rather than a documentTrigger, pressing
escape will not close the editor if done elsewhere on the page, e.g.
in the site search bar, or an unrelated modal.
Move the logic to ArticleTarget as this could theoretically work
on the mobile site with a keyboard.
Allows us to remove some hacks around the ToolbarDialog that are no
longer necessary.
The command can also be used by the MWBackTool (which should be renamed)
and allows us to remove some custom logic from it.
Bug: T310694
Bug: T310695
Bug: T306763
Change-Id: I91ee6916a91d80d9872b3b44dad7eca55ad1acc4
Depends-On: I29f6af4cc7c71a63a6d1bafc53d16b9abd1b60ec
This patch adds top and bottom margins for each transclustion
part, excluding the top of the first element and the bottom
of the last if those are non-indended ones (ButtonWidget).
Bug: T309584
Change-Id: Ibdcf1293676bd59c7868f3d197805c8b9e743cdc
The testing phase for the implemented features is finished. So the
feedback link for the project can be removed.
Also added missing documentation for a message key used.
Bug: T307188
Change-Id: I2e2e4ff58d2bacda5ae841bcf6f418e786a3967e
For some use cases of the transclusion dialog such as Insert
Citation, the back button is inappropriate and we want to keep the
close box.
This partially reverts commit 2e2e40e76e.
Bug: T310602
Change-Id: If68a822c260618c0a93eb8d0c46d2b452fee8baa
I benchmarked the template dialog before and after the change made in
I0f35a19 with a large 200 part multi-part template. I measured only
the time spend in .updateSize().
Before I0f35a19: 3.6s
After: 4.6s
With this small fix here: 3.6s
Bug: T309875
Change-Id: I2c2892e173ba70c746fb71624c65b7f4ffde4419
I'm more in favor of leaving no garbage behind. The TODO with a date
is a good way of making sure this gets removed eventually.
This could have been part of Ie6eea76. The new code is added to the
same spot where the code removed in Ie6eea76 originally was.
Bug: T296471
Depends-On: Ie6eea76dacdc614ecb910c48e7e1f519b8c69322
Change-Id: Idec63201ff4aa52a0c53c6d007577a93c94e0ec0
Following the MediaWiki changes from T301203, we should use
the messages 'skin-view-edit' and 'skin-view-create' instead
of 'edit' and 'create'.
(Also remove redundant definitions in extension.json, we load
all messages listed in 'VisualEditorTabMessages'.)
Bug: T310529
Change-Id: If055fa2a4dc009be869425e6c2262c9b62056179
The "mediaClass" property now only serves to capture the original class
found on the media so that it can be roundtripped without causing dirty
diffs. In the 2.4.0 version of Parsoid's output, that will still be
the usual Image/Audio/Video. As of 2.5.0, it will always be File and
the mediaClass property can be dropped.
Parsoid is currently forward compatible with serializing mw:File, so
edited or new media can use that type already.
The contextmenu item for media has been updated to make use of the
"mediaTag" instead of mediaClass to continue distinguishing media types.
That was the only place a grep of mediaClass turned up any use.
Bug: T273505
Change-Id: If5dc6b794dacd6973d3b2093e6b385591b91d539
Before the (intentional) design decision was to not do anything special
when the same parameter is used multiple times (via aliases). Garbage
in, garbage out. Only the first usage of the parameter would work as
intended. The rest was ignored and subsequently removed from the
wikitext.
New design decision: Track and display duplicates as they appear in the
wikitext.
Notes:
* It's not possible to create such a situation in VE. Do this via
wikitext.
* Labels will be made distinguishable via T309198.
* Possible warning messages will be added later.
* The behavior when unchecking a duplicate will be specified later.
Bug: T309198
Bug: T310248
Change-Id: I6011344638cdad8529d8f57513ef51b5237eb878
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 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/T309013#7953227
Bug: T309013
Change-Id: I7627f116cf32ceb3455a33f4f7bb55208ba92671
Parsoid stopped emitting figure-inline in content version 2.2.0 and VE
requests 2.4.0, which isn't satisfied by the earlier versions.
Change-Id: I5f47fbe85fdca7fe429952709b62f5d1cc548daf
This only happens in AMC. Currently it is also not possible
to update categories in VE, but this may become possible if
we enable 2017WTE on mobile.
Change-Id: Ifeb6cc18910ce2fca634bc3e2245aac7e5c37e52
The previous comment was true, but we forgot another edge-case. The
same parameter can appear multiple times. The old code tried to
delete it 2 times, which deleted something else.
Note that the behavior for duplicates is larely unspecified. We
will work on this soon. This is only a first quick step to fix this
specific bug.
Bug: T309203
Change-Id: If0afb2c19626c3d9db0d109d6559ae74698ed378
This avoids a meaningless attirbute change with the image
is unmodified (as thumbUrl can be a different size).
Change-Id: Ib79a4703382552e38022a3f345ca5cd762c52303
Needed after Ia18f31a299338f94e69f1882e6e477f3a22ae905 in VE core.
Bug: T307849
Depends-On: Ia18f31a299338f94e69f1882e6e477f3a22ae905
Change-Id: I87f3ac0974702ecaf7f5459604371de06f4a5756
Now done in VE core since Ie86217ba5651df8c427464e460ed836903834a3c.
Bug: T308200
Depends-On: Ie86217ba5651df8c427464e460ed836903834a3c
Change-Id: I3f65b0bc100895f7bd2262c3cbb5231fb055758b
This lets us begin the import from another page, for example an edit
tab or button.
If the CollabPad page already exists, it will be opened without
overwriting with freshly-imported page contents.
Change-Id: I89293b9b06253243826d8f8144a5e93d55856e7c
* Implement ve.dm.MWGalleryImageNode.static.isDiffComparable to
match ve.dm.MWImageNode, in that images with different resources
are not compared.
* Diff galleries as documents so remove/inserts are rendered.
Bug: T308747
Change-Id: Ide6f4110e65cad7f6bb6d13766815413602fd991
Visual Editor currently requests MediaWiki DOM version 2.0.0
when talking to Parsoid. Since Parsoid treats that as a request
for 2.4.0, its current version, and Parsoid doesn't have a
2.4.0->2.0.0 downgrade path, passing the latest Parsoid version
(2.4.0) should make no difference in practice -- but would better
match current reality.
Change-Id: Ia2bc0c1981db6f573a69fb1910cef4304c80ae00
Now that Parsoid's ServiceWorkers have been merged to core, this adds
support for "zero configuration Visual Editor" to the master branch.
Like earlier zero-conf work, this does not use RESTBase for stashing
or for reliable selective serialization. Future integration work
with ParserCache will reintroduce this functionality. Nevertheless,
this implementation should have feature parity with the "loopback interface"
zero conf VE we've been shipping since 1.35.
Bug: T305108
Change-Id: I7b5b4a6d16b07914f947cbaf498ad1d3cf2447a5
This patch fixes a bug which causes long parameter descriptions to
be to non-collapsible when adding them from the sidebar-only view on
smaller screens.
Change-Id: If373587a9b2c3841ad6814f74bfcbf0c0f013488
This is done in preparation for Iac6205f to make it easier to review.
This patch here is meant to be a no-op that doesn't change anything,
just moves existing code around.
Bug: T307986
Change-Id: Ie6ccd9528e5799ba340fed344e1f47a443c2c51f
This is currently only handled in DesktopArticleTarget in teardown,
which happens after we've wasted time trying to update the page.
Also ensure we always reload on non-view pages on MobileArticleTarget
for other types of teardown (e.g. quitting the editor).
Change-Id: I7fb352fcacc8727bb113115e98af38a3940a8f9c
Follow-up for what was done in I37505af. I don't fully understand why
these two extras are sometimes needed and sometimes not. But this exact
combination of 3 properties is used in multiple places already, so
let's just do the same here.
Bug: T300008
Change-Id: I1eba5fc378475d365111add58a141c3114dc0118
This focuses on some scenarios that are
a) complex enough to be worth a test,
b) but simple enough so I don't need to spend hours on comming up
with a test setup. ;-)
This patch also simplifies the ARIA related code in
MWTransclusionOutlinePartWidget a bit.
* Check 1 of the 3 ARIA configs only. Only having one is already
helpful and should not be skipped.
* No need for the large conditional. setAriaDescribedBy() works fine
with undefined.
Bug: T291157
Change-Id: I142782ec9b96147de64497f4f6a373eae05b9c8e
The idea of the code under test is that it combines the results from
2 different API requests in a specific way. This is a first basic test
where both search results are small.
Bug: T291158
Change-Id: Ic4de57fb6b85afb952ea604769fddd06d44814c0
Will implicitly set a max width for the button due to the margin.
Long words will break in the middle of the word.
Also includes a shorter label.
Bug: T300008
Change-Id: I37505af8383d8c0c2bd4af3987ec5e2a3049688a
Will be removed when parameters are added. Needs different margins
depending on beeing shown in the single transclusion mode without
header or on multiple transclusions.
Bug: T300710
Change-Id: Ieb95d7276aa4d4b0fcbb74f87ab734e4a393dc21
Requires switchToFallbackWikitextEditor to return a promise.
We can now pass dataPromise to the progress bar shown when
switching editors, so it hides if the switch fails.
Also fix logic for when a failed load is not retried.
Bug: T306763
Change-Id: I752ca505e7957b392202d44455b1e21b6e50fa63
If links get pasted into VE and they lack a "//" in the `href`, they
are automatically considered to be "internal", thus being converted into
`[[...]]` wikitext links.
In case of pseudo protocols like `mailto:` this should not be the case.
This patch uses MediaWiki core settings to check if a `href` value is an
external protocol known to the application.
Bug: T297575
Depends-On: I2e584f6d5adc6b2d735e79cea64f2beeb5f2c36d
Change-Id: I2b383106450e02cc6bcc1b99d547ff2ed7832b4c
Before, the fallback algorithm was somewhat adaptive, trying harder and
harder the fewer CirrusSearch results have been found. This updated
algorithm guarantees that 10 results are shown.
Warning: You might see only 9 results. The reason is a bogus, unrelated
behavior in the mw.widgets.TitleWidget in core that's used as a base
class here. There is a "showMissing" option that's apparently enabled
by default, and prepends a non-existing title from the main namespace,
ignoring the "namespace" option. This extra result from the wrong
namespace is later dropped by the very same widget.
This code here sees 10 results before the bogus one is dropped.
Disabling "showMissing" causes other issues. We would need a series (?)
of custom hacks to work around all this, but this seems inappropriate.
Let's live with 9 for the moment.
Bug: T303524
Change-Id: I2c577c9ef2752b6c6cd360f4023e151e9272fcd5
The main advantage of this change is that it drops the assumption
that the index starts with 1. This is not necessarily the case when
we prepend extra search results. Dropping this assumption here allows
to simplify such code.
* The incoming list of pages is guaranteed to be an array.
* There is no point (any more) that could cause the array to become
sparse.
* Note we still make a copy of the `origPages` array at some point,
e.g. on `.filter()`.
Bug: T303524
Change-Id: Ifbd92bb052155c613d2ca21ab6d54a0b3ef28c0c
New changes:
72bd71dbc Localisation updates from https://translatewiki.net.
b7f38d993 Re-apply "Fix for showing "invisible icon" when root node is hidden""
Local changes:
* Ensure templates are attached to DOM before rendering invisible
icon in converter hack.
Bug: T305110
Change-Id: I0e4c02619b2a06b9932f214ae482093fdb0cb22c
This option is not only buggy (it just doesn't work when the namespace
option is set the same time), it is not useful in this context even
if it would work. It doesn't make much sense to suggest non-existing
templates in the context of the template dialog. If adding a
non-existing template really is what the user wants, they can still do
this by simply typing the name of the template and submitting the form.
We never need this to show up in the suggester below the input field.
The main advantage of this change is that is saves 1 useless API
request that's potentially done every time a key is pressed.
Bug: T303524
Change-Id: I903340a06d6e6490bb58f628f41903aa044ccb21
This separates the two steps:
1. See if items in the list of `origPages` miss their `.index`
property, and add it if possible.
2. Later code doesn't need to care about redirects any more.
Note that `origPages` is not used for anything else. And even if,
it's not wrong to have the index for each search result on both the
redirect and the redirect target.
Change-Id: I12135f0430c944b4e33c49ece7779d7c3bb6c211
This is not a pageid, but a simple numeric index in an array. Luckily
it doesn't make a practical difference for this particluar way of
iterating something.
Change-Id: I7ec9ace00d4fba7adde17670058a0365b30f5617
Such comments don't add any new information. The method signature
alone already tells the full story.
We did this already to a lot of the template related code we touched.
This is just a bit of cleanup to make it consistent.
Change-Id: I932b620910924a16dc0d31d6c8a3ab11818316fe
The numeric part of these ids is never used on it's own. There is no
need to expose it.
Note we renamed the method not long ago in I6eeab8b to reflect better
what it does. This is the next step. We just forgot it back then.
Change-Id: I5da82855e99ea3a42a5d91379c6974ae9c154518
The result is guaranteed to be in formatversion=2, where the list of
pages is an array, not an object.
Change-Id: Ic73a68c3e249a70108a6a19a89f4ff6c475794ed
The goal is to have some tests fail if a new property is added to the
API result, but it's not stored in auto-save data.
Bug: T305225
Change-Id: I9ac328c71c616a8f33e1cfb39431cd9a665adafc
The refactor in I0eaeb987 broke this feature by moving
the call that modifies the response object to after it
is passed to the target.
Change-Id: Id98c1ccabde478540af34ed3356f319ae336590a
New changes:
91a762803 ve.dm.ModelRegistry: Disallow unknown types on tag and tag+func matches
8575e8ad8 PreviewElement: Document event
d24f96874 Allow unknown types for ve.dm.AlienMetaItem
106354b38 Follow-up I534ce003d: Fix height restriction calculation of desktop context
Local changes:
* Allow unknown types for ve.dm.MWExternalLinkAnnotation, it is meant
to match all <a> tags and handles unknown types in toDataElement()
Depends-On: I785be75f389136769084a8e14832a09e3a519fc1
Bug: T303298
Bug: T304847
Change-Id: I75d1e82a43f5f96a9093cd334c524527918efaa5
This was added to make the context a bit more consistent
with the regular link inspector, however with I2fec865570
we making these delete buttons strictly mobile only.
Change-Id: I52936919e332aee851ccd11a862367c97eb41b39
This code is optimized for the 2 most relevant use cases:
1. When Cirrus finds 10 results, we still want to search for the top 1
prefix match. This is critical for templates like !!. This will appear
at the top. unshiftPages() makes sure the limit of 10 is enforced.
2. When Cirrus fails to find anything, we search for 10 prefix matches
and use these instead.
The code can also handle everything in between. For example, when
Cirrus finds 5 results, we search for 5 more prefix matches and add
them when Cirrus missed them. The total number in the end might be 5 to
10 depending on the number of duplicates. This is intentional. Why?
Let's say we always search for 10 prefix matches and add them to the
top when Cirrus missed them. This might remove _all_ Cirrus results.
This shouldn't happen. This extra code is only to fill in glaring gaps,
not to replace Cirrus. 5 results are fine.
Bug: T303524
Change-Id: Ib0471795124c0c7001b6901edaf8e7b380e426b1
There is already some kind of "fallback" to prefixsearch. We always
check if the top-1 prefixsearch result is part of the result set.
Because of this the current worst-case scenario is that only this
1 result is shown.
This patch implements a full fallback to prefixsearch. But only when
there are 0 CirrusSearch results. Further tuning might be done in
later patches.
Bug: T304925
Change-Id: I1927eedad60c9b9ac2021481a85376c08ccf6fdb
In detail:
* Just compare strings where possible. Sometimes the first character
alone is distinct enough.
* Add missing ^ anchor to avoid possibly running unbound pattern
searches. This is not strictly necessary when we know the input is
fixed-length. Still I find it more robust and more obvious what it's
meant to do. Not to "search for some match somewhere" but to check if
the input _is_ a match.
Change-Id: I9b7651d8ec1266521c22b38ccabb95a947903f72
.test() is the dedicated syntax for a boolean "does match? yes/no?"
check. .match() returns an array of matches, or null. This is just not
needed in these situations.
Change-Id: Ibb996ab843d1a6c7d7af98d6a112990665d543b2
Also invert logic and rename variables to make tests in tryTeardown
and switchToWikitextSection identical.
Bug: T294463
Change-Id: Icb86a676c3f315e4a3d229f46eb383dcd7959973
We ended mixing two concepts in a single method:
* We need a method that allows us to create the parameter list widget
when it's needed, even if it's empty. This is relevant when a template
is entirely empty, and the first parameter is added. This wasn't
working. Instead the parameter list was created with all parameters.
* On initialization we either want all parameters to be shown, or only
used ones. But this code is only needed once, on initialization. I
ended inlining this code in this patch.
Bug: T300640
Bug: T304046
Change-Id: I6620a870e4420dcb8fecf522b3274458eeec891d
A class, mw-broken-media, was added in
I92359048b42d32fe8a0f2cb79cd348cf5f2c56cc
Bug: T304010
Bug: T270150
Change-Id: I542d7d879868c9030ce00b2686cdfd0077e49491
These two lines forcefully enabled the review/preview buttons, even
when the review/preview panel is already active.
Bug: T300448
Change-Id: I6dbe6ee88728a65233a455b768f17bff668fe3a8
In the mobile view, parameters don't have left padding so some styles
should only be applied to the desktop stylesheet.
Bug: T304167
Change-Id: I1846512c21aae36f212fe142b7d96ac91e46854b
Currently in VE desktop, items which can be deleted with
delete/backspace do not have this button. If that situation is
considered a problem it sould be solved consitently for all
focusable nodes as part of a wider fix.
Adding a delete button to template contexts makes them inconsistent
with other focusable nodes.
Bug: T274263
Change-Id: I2f7508a605852274ba8f40b2afd1dfd56600aa36
This fixes a styling issue with inline descriptions, for the scenario
when the inline description feature is enabled but the new sidebar is
disabled.
Bug: T304167
Change-Id: Ida4da4605da5143de2a27725d87d5876aea7065c
The Apex skin (not to be confused with the Apex OOUI theme)
is abandonware. Keeping files around that pretend to support it
is just maintenance debt.
Change-Id: I176e4e25307ab8ca4166a6e9b55e3bec4f058d05
Parsoid added a class and, without it, we get selser complaining
about wrappers being modified, similar to T214649.
The "image" class is removed since Parsoid never added it (although it
now has "mw-file-description" for a similar purpose) and the legacy
parser doesn't apply it indiscriminately.
It doesn't seem like VE supports editing the |link= media option; it
just tries to roundtrip what's there and drops it on edit. The patch
here works with that limitation.
Galleries are found to drop href's, breaking selser, and should be fixed
in a follow up.
Bug: T292657
Bug: T303469
Change-Id: I92359048b42d32fe8a0f2cb79cd348cf5f2c56cc
Due to the stickyness we need to scroll the header and the first
visible item from the parameter list to have the best result in all
cases.
We also only want to scroll when the user triggered hiding the
fields. Not when the template outline view is initialized.
Bug: T302965
Change-Id: I84d293888a7dbf13ec655c293c0fc3a79edca698
It removes some indirection.
* Find: \$\( '<.+?' \)\.toArray\(\)
* Replace with: $.parseHTML( \1 )
(also replaced a few by hand that weren't caught by that regexp)
Change-Id: I2639cc4a26bc960046a189504dd8058344e14d60
New changes:
cf7b2a9c4 Restore margin between wrapper paragraphs and document edge
629109b6b ve.ce.Surface tests: Sometimes assert originalDomElements in paste tests
3a5bd323c Use wrapper paragraphs in fixUpInsertion
Local changes:
* Update tests to ignore some internal properties
Bug: T299815
Bug: T299817
Change-Id: Ie52348f62320eb135c2f950ac78a77d9c665fef2
This was added back in 2020 so mw:DisplaySpace would work while the
RESTBase cache still contained old data. It's been two years, it should
be good now.
Follow-up to ed3579245e
Change-Id: I65665231b78198633682d52c48068c89807df55e
Once we determined a link is internal from the first regex, we don't to
check if it matches the wgScript path as well. This would cause
"/index.php/Article" style paths to be detected as external as they failed
this second check.
Change-Id: I560d8080c513c523c68f2750be332e9fd91de192
This logic has been moved to the relevant extensions which can
now disable VE using a hook.
Depends-On: I47880be15b6ce1a93f389a32aff304cc3b798bcb
Depends-On: If188e8fcceb248738fc625ddd5afec351c01c484
Bug: T174180
Change-Id: I245295373af3caffb1d4cbc288e8d2bd008520b7
* I can't guarantee this fixes T301914. But I suspect the bug can not
be triggered any more with this plus If9b6050 in place.
* I wasn't able to reproduce the issue locally. But I learned that
1. it's related to the hotkey, and
2. it's because the dialog looses focus, and the focus ends on the
VE surface in te background. Pressing Esc there closes VE. That's
intended behavior.
* I tried to use .trigger( 'click' ), but it doesn't make much of a
difference. The only magic check is if the button is disabled. But no
visibility checks are done.
Bug: T301914
Change-Id: I2f66fc2411144c60cd08baae58452d336b4e9802
In a test case with 200 templates where all but a few parameters are
unused the loading time is cut in half.
Bug: T300974
Change-Id: Ice850cb9e5e95b9e3a19ff511b3a4f32117c7199
Our theory is that browser plugin spam shouldn't reach
the DM document unless we've made a coding error like
I194ff1d57. Log future errors so if there is more plugin
spam in the future, we can investigate the cause.
Bug: T298147
Depends-On: I705195bc5d0f76a38da5d2cc09fab184a2c32401
Change-Id: I3403859906ceaa51be63b0d79f474f0289ab4408
Same random finds while working on something else. I carefully
checked and made sure these methods are actually called without the
optional parameter.
Change-Id: Iab36fd130258322985b5d6e7f8e1f7b4ee235ba2
This focuses on a few trivial cases where the syntax helps making
the code more readable. One level of indirection is gone with this.
Change-Id: Ibf25d7eaa06952e69b36bd5a78a48d04ac62890c
These are only needed when we need to access a specific `this` from
within another `function () {}` context. This is not the case in the
situations here.
This is split from Ibf25d7e to make it smaller and easier to argue
about.
Change-Id: Ide1476de91fc343aa992ad92a1321d3a38b06dd0
The `.` character class matches any character *except* line terminators,
but edit summaries can contain those. Use [^] to match truly everything
in the comment part. (In the section part, I assume `.` is okay.)
Bug: T302103
Change-Id: I29fcdd7489d118674bab5cfe5c0a15b8e4efac64
Passing the useskin parameter ensures that output hooks are run
on the new page HTML. This already happens because we request
the 'subtitle' and 'categorieshtml' props which also trigger
skin mode (along with the 'headhtml' which we don't request).
However it is better for us to be explicit that we want the rendering
for a specific skin, rather than relying on these props to trigger
the correct mode.
Also pass through mobileformat param, which is added by a hook
in MobileFrontend.
Change-Id: I1cd2c5c5c13ae0b90cc32e441b453532343a434a
This ensures the full loading sequence is shown when the
user opens VE using history navigation.
Bug: T301843
Change-Id: Ia7a641c8bd5a036f23c9da94bc539d8cf66c5021
Previously we were adding an event listener every time the
target was opened, and not removing the old ones.
Change-Id: I0ce609f1d9e2d6fb00b605dcade6f27e7a887b9d
Found via Iaf989bc50363a (patch set 6):
> FAILED TESTS:
> ve.init.mw.DesktopArticleTarget
> ✖ init
> Died on test #1
> TypeError: Cannot set property 'pageLanguageCode' of null
> at Object.<anonymous>
Bug: T89434
Change-Id: I8ad2cc4c115224eb276d1488091b9518fde739f9
* Append the toolbar before starting the scroll
* If the toolbar is floating set the anchor to full height
immediately. This shouldn't cause the content to jump down
due to scroll anchoring:
(https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-anchor/Guide_to_scroll_anchoring)
But add our own logic for it for browsers which don't support that.
* Now the browser only has to deal with the scroll animation,
and not the height animation of the toolbar anchor at the
same time, making it smoother and less buggy.
Bug: T301773
Change-Id: I61d533d40758d559b03c858e0006ef2e4f0fcd16
Previously, a toolbar with the extra tool could be shown for a moment
while the editor was loading.
Bug: T301626
Change-Id: I7f5d01ef6e64ef85141a38a236fe2a1d1ada00e8
When the editor is loading, and the browser window is narrow enough
that we should use the narrow toolbar, the wide toolbar would appear
for a moment. It was particularly jarring when it caused it to wrap
over two lines.
Bug: T301626
Change-Id: I551a66edab40235c4a26751eed163b5b30ef698f
Make sure it works on pages which don't trigger DT, such as full-page
wikitext editing.
Bug: T301497
Change-Id: Id3d46eef319f278ee71fbbedbddd16d5ffd540d8
* Pass 'noPrompt' to #tryTeardown
* #loadFail gets triggered by the data promise being aborted,
so if 'activating' has been cleared at this point, don't
prompt about there being load error.
* Retry/cancel dialog assumed you were coming from a wikitext
editor. Rewrite the logic to handle teardown the target if
the user is coming from read mode.
Change-Id: If9e5d1fbf00f8359f4d06fe7fc3064cc7e349bc9
I bit more logic was needed to make sure, the state of the widget
resets when all unused fields are added during search.
Bug: T299811
Change-Id: I3006c233fda5490e323bc3a3e631bf0c1199bda3
Covers the base functionality to hide the parameters. Performance
optimizations could be done in a follow up.
Bug: T300640
Change-Id: Ia99b5da392273f1445e475a0720a656460612dcf
The behavior is now consistant with what would happen when the
buttons are triggered.
Instead of emitting a button click I directly wired the methods
that will by triggerd by the click. This might make it easier to
remove the old sidbar later.
To avoid movement when the buttons should be disabled, an
additional check was added to the onMove method. It's not identical
to the more complex check in the outlineControlsWidget, but should
be enough for our use case. The onDelete method already just does
nothing if nothing is selected.
Bug: T300971
Change-Id: I8a278c9657c91fd648944b5a8c1204c9fff75b7e
This option was added in 0.43.0. Now that the close button is handled,
the remaining functionality (store a flag in local storage, and fixing
link targets) doesn't really justify a separate class, especially as
it's currently only used once.
Change-Id: I0fd81cadccc077dbf957302f9f41409c5a1f4f20
Previous attempt in 005a8d24ef,
reverted in 3c1d167b33.
The deduplicateStyles() function lacked a check for fosterable
positions, which caused T299767. This is now fixed.
Also added tests.
Bug: T287675
Bug: T299251
Change-Id: I0d22be9b66d26d09373cee63dd6ce52c1659e62d
This prevents an unwanted scroll when the main edit link is used
and the first section is below the viewport.
Change-Id: Ib99ebe2dc5c105c3fcbd687ef5740166267f5536
This doesn't change the position of the loading bar, but it keeps it
still if any scrolling happens. Previously we expected the user probably
wouldn't scroll while VE is loading, but in subsequent commits we
will trigger automatic scrolling in section editing mode.
Change-Id: I1404ccd77583d808ef79291c6cb4f561e76bd41c
We introduced these methods in Idc0cccbe but didn't update this
logic in MW.
This prevents focus being put inside an image caption if the section
starts with a block image.
Change-Id: I4cc1a116c1dcc8e1c642359ea768b6f5265a061b
Parsing it in the RL module caused the module cache key to depend
on the parse, which is slow and makes ResourceLoader sad. The usual
approach for solving this (I206bb05d28) can't be used, because of
how EditPage generates this message.
Bonus #1:
Generate the message for the correct page title. MediaWiki allows
customizing it per-namespace or even per-title, which we haven't
supported before.
Bonus #2:
Pass the context for message localisation (depends on I5f7c77970d).
EditPage::getCopyrightWarning() was parsing messages without the
interface flag, causing some needless processing elsewhere.
Depends-On: I5f7c77970d0525c0ff394f8bd72c69dcb5d00623
Bug: T298822
Change-Id: Iaa626f0e6379a5a370f9c465cea8528bb5bde7f7
New changes:
e5963f084 DiffElement: Remove doc-child-change wrappers
90a359f79 Follow-up Ia593dad8: Use modifyData when sanitizing
f5c7f0f20 Localisation updates from https://translatewiki.net.
Local changes:
* Update tests for the removal of doc-child-change wrappers
Change-Id: Ib29f23625de9151d50e0f3b7796b15ca9ed89133
Do show a confirmation when editing templates without parameters,
but don't show the confirmation when you're only on the template search
page.
Bug: T299394
Change-Id: I8205bb6a0f92b7b535a80f7f6a43674f1aa5b51b
We were using addItems() incorrectly, and it stopped working after
change Ib1241f93d214e8a7d2f757a120f84de3879575af in OOUI.
Bug: T299818
Change-Id: I2d00cb929673a419789ca6f8bc308721373e1da8
The only places where this dialog is used now will have the same
button labels and format. We want to use "normal" destructive here
so no additional "primary" styling.
It seems that the focus on the action will only be applied
automatically when it is primary. So extra code is needed.
Bug: T299647
Change-Id: Ib5250b79e85d27ea197b83c6380863d0749e5d89
The search field is of not much use when there is not really anything
to search. It wastes more space than the actual list of parameters.
Approved by UX, see T298259#7626538.
Bug: T298259
Change-Id: I01784a1c463d8b0b504897b20179719f91597d19
For most nodes, isDeletable() returns false on desktop and true on
mobile. Change e0bbf0b979 meant to
change the desktop version to depend on the new config settings, but
accidentally changed mobile too.
Bug: T298828
Change-Id: I8ec5e8031a27eee18c16dfcd267937cc76722c50
Double clicks have been tracked as "window-open-from-command", but are
now tracked as "window-open-from-context". This patch changes this
back.
Bug: T297773
Change-Id: Icabc5db526c35d681d077043c54ca009b2296bbe
The first click opens a little context menu (…ContextItem in the
code). This patch makes a double click trigger the "Edit" button in
this context item instead of executing the "open window" command
itself. This is needed to trigger the "Loading…" code in the context
item.
Bug: T297773
Change-Id: Ie1daa2e6b460c5c506a847bd767af2cb48e73a61
From the user's perspective nothing changes. A template without
parameters doesn't show anything. (Technically there was an empty
<div>, but it doesn't do anything.) The moment the first parameter
is added the required sub-widget is created and available from there
on.
This saves loading time and memory, especially when a multi-part
template contains many templates without parameters.
Bug: T298259
Change-Id: Ib1bd2cd0be4fece4acc92e6e5f63133a7986cf81
The checkboxes are actually indented a bit more than the search box
with this change the extra indentation is applied and the hide
button text is alligned accordingly.
Bug: T298259
Change-Id: I5508ab883c23e7285c023dc127529ffa4dbe58c7
From the user's perspective this is the same as before: When a
template doesn't have any parameters, there is no search field. The
moment the first (undocumented) parameter is added the search field
appears.
This is just delayed now. The widgets are only created the moment
they are actually needed.
This saves loading time and memory, especially in a multi-part
transclusion with many zero-parameter templates.
This also makes it a lot easier to change the minimal number of
parameters from 1 to e.g. 4.
Includes reverting the flexible header composition done in
Ib050e30a50ef965c1524e977d3a600c3ff836774
Bug: T298259
Change-Id: Ied7541d8d5c0b478a439dd31ce072e634287f181
The search field will stay at the top of the window while scrolling a
single template. In multi-part transclusions, the header will also
be sticky.
Hides the template header in single-part transclusions.
Bug: T298618
Change-Id: Ib050e30a50ef965c1524e977d3a600c3ff836774
Reuse the back button confirmation dialog for the close button. The
condition is slightly different: need confirmation if there are any
manually-entered values for any parameter AND the user has edited
the template in this session.
The "reset" action was synthetic, only used internally and not
connected to buttons or menus. Canonically, action='' is the close
action for OOUI.
Bug: T297792
Change-Id: I4ff644c7ab24ed9ba1a4c27d762563c5d6771cfc
* Find the first section below the top of the viewport
(usually visible) and measure its offset.
* After loading the editor, ensure this heading is still
at the same position on the page.
Bug: T296910
Change-Id: I9a05ea74ba3c19a4a91ddc1bc0afe311851c53e6
Without changing behavior, consolidate the logic for detecting
whether the editor has made changes to the template. This is
responsible for enabling and disabling the "Apply changes" button.
Change-Id: Ic4755b13f30fb738a7cb1eebaddef0435ea61d34
Currently, the insert template dialog includes a back button in the
upper corner. Confirmation of abandoning unsaved changes was
accomplished in an overlay panel. This patch rewrites as a dialog
and updates the on-screen text.
Bug: T297792
Change-Id: Ifa2ff97c9284609ee2a784f455789c56a762ba50
* Use this.getElementWindow
* Use this.surface.padding to avoid race condition
when the toolbar hasn't fully rendered yet.
Change-Id: I055b1d9458d73e435ede6096941a3e72c9c1ce74
This prevents your preference being changed if you just
followed a link with a diffmode parameter.
Change-Id: I755563bde285e95c0367119d49a40e1dd3c5e178
This is guaranteed via ve.init.mw.Target.getContentApi(). But the
ContentTranslation extension replaces this, and does not set a
formatversion. See e.g. SectionTranslationTarget.getContentApi().
Bug: T298599
Change-Id: I8768cae3153e9cbc29a8796ec21ef249f80471ed
In case the save is triggered without the save dialog (for example, when a null edit is made during suggested edits task in GrowthExperiments)
Bug: T298552
Change-Id: Id49b967cfa52d33848e9c911086000fa4501fa7f
It's not a getter, but a generator. I found the name confusing.
Getters typically don't return something different every time you
call them.
Change-Id: I6eeab8b6a8644e430003f6e1ad77ab4b28e0d8c9
Prevents accidentally treating plain text or user input
as HTML, which could be an XSS vulnerability.
Change-Id: Id4af48447a0907962a57340cb60aca08df9cc505
* Fix incorrect use of .append() instead of .text() (which was causing
some l10n messages to be treated as raw HTML)
* Avoid escaping and parsing HTML several times when plain text was
intended
* Remove some unused options and variables
Follow-up to 839b64d882.
Change-Id: I124257c73fe09713afefccdec8e90200e6ae433d
The "Add parameter" page always starts collapsed. Even if a template
doesn't contain anything but this. But most of the content isn't
visible, unless the user presses the button. It's not only a lot of
content, it's also rather expensive, including .parseDom(),
LinkCache.styleElement(), and ve.targetLinksToNewWindow(). This adds
up in large multi-part transclusions. In an example with 200 parts
the total blocking time goes down from 2.9s to 2.4s. Which means this
is not a major bottleneck, but still worth it.
Bug: T296335
Change-Id: Ieab9fd35d145142b04d2267d8e5a2e10a4c02784
* Make ve.ce.MWBlockImageNode autofocus=false, remove
unused transition property
* Remove ignoreChildren from ve.dm.MWBlockImageNode
based on new definition
* Remove tests which assert that deleting in a list next
to a block image always de-indents. If this is desired
behaviour it should be fixed without reference to
ignoreChildren.
Bug: T295905
Depends-On: Idc0cccbe73d1b49d07b60c14a192a40f47d64608
Change-Id: Ib79a070f5d36dbe7742fa0760f8cdf55fe3046ed
These pieces are only relevant when the new "inlineDescription"
feature is enabled. In other words: This can't have an effect on
the old dialog.
The 2.5em left and right are from an old styling when the
parameter pages have been indented.
Change-Id: I022b0dd94ee66f7de114c055c3f453317a7f6131
This sorting algorithm was introduced via Ic6bc348 (T274544). Note
there is no index parameter in the .onReplacePart() handler at this
point. When a part was moved, it was removed and simply appended
to the end. The additional sorting was needed to move it back to the
correct position.
This changed a few days later via Iafe29f1. There is now an index
parameter. The .onReplacePart() handler does the same as before, but
puts the part at the correct position right away. The additional
sorting is pointless since then.
The removed code alone is responsible for 1/3 of the total blocking
time when the template dialog opens.
Bug: T296335
Change-Id: I6c3fa70b532d34cd29d59c3b48ab81ebf608d548
onReplacePart is being called when templates are being moved up or down.
To prevent that the multipart message gets moved around passively e.g.
by one element being added above it, this patch adds it to the top after each movement.
Bug: T292829
Change-Id: I946c9bc4ba5e1d261aefbb28a8c642bb58964842
We forgot change the way that message is build while adding the link
in 07f105fd7. Now it gets parsed correctly and the link will open in
a new tab.
Bug: T284985
Change-Id: I1ed9dfdafd08d08c5aff45f4b74c540b35ec14a1
This is only relevant with the old design. It's only noticable when
a field shows all 3 action icons: info, raw wikitext mode, and
trashcan. The last icon can wrap to the next line when the screen
is very narrow.
I tried to apply nowrap, but this causes other style issues.
Removing the arbitrary width allows the action container to be
as wide as it needs to be. I can't tell why this restriction was
there. It is in no way necessary, as far as I can see. I can only
guess it's a temporary artifact from when the dialog was designed.
Bug: T296730
Change-Id: I77129ccc3afe002ba697b1787b41d0a388d5f4b8
This does have a significant impact on the performance of the
template dialog. Not only on construction time, but also because
MWExpandableContentElement objects do some quite expensive
.updateSize() calculations the moment they become visible.
I profiled a template with (only) 200 undocumented parameters.
Construction time goes down from ~600ms to ~520ms. The mentioned
.updateSize() runtime goes down from ~300ms to ~10ms.
Bug: T296335
Change-Id: I280f814e722b299aae0ec6a5a2fa59292e3e5887
This doesn't have much of an impact on performance according to my
profiling. But I think it's worth it nevertheless. The idea is to
skip that <div> entirely when it's empty.
Bug: T296335
Change-Id: Id155725fbc2e3453acc1cdcabfdc2d687285d694
In OOUI the close button is always on the left side. See
https://doc.wikimedia.org/oojs-ui/master/demos/?page=dialogs&theme=wikimediaui&direction=ltr&platform=desktop
The CSS hack to move it to the other side doesn't work and must be
removed. You can see the problem the moment the text is longer (which
can easily happen in translated versions).
I tried to come up with a more official way to move the button to the
other side, but gave up. One way is to replace the existing
flags: [ 'safe', 'close' ]
with:
flags: [ 'primary', 'close' ],
framed: false
But this causes other style problems. Let's remove the bogus CSS
first and possibly try again in a later patch.
Bug: T294839
Change-Id: Ia6ddefd99e4a03a87b0450ab94712ff19bb268e4
There was a remaining issue when the window was made very narrow in
desktop mode (smaller than 500px). This patch doesn't aim to really
"fix" the dialog's design in this case. The goal is to make the popup
window appear less broken, so the text can stil be read and the
buttons clicked. That's all.
This patch should not have any effect in:
a) mobile mode,
b) desktop mode when the window is wider than 500px.
Bug: T294839
Change-Id: I3171dbb991533b91eaadba63b78d0ff40aa486dc
Same as I980b72c, but with another shortcut. Again. We tried
Ctrl+Shift+X before (conflicts with RTL feature in VisualEditor), as
well as Ctrl+Alt+D (conflicts with "hide all windows" incUbuntu).
Bug: T294905
Change-Id: Iae7ba759fcd7c107ef586bd5d9ae3cdbe445cedc
Allows setting aria labels and descriptions on elements in a
convinient way. I did not use the the .mixin. convention here for
because there's already another mixin in that folder that's also
not having .mixin. as part of its name. And then there's also no
no need to open up that extra namespace here.
If we move this upstream at some point this can be changed though.
Bug: T291284
Change-Id: I1b3d40400d539f851f13719e16ced200968a7f92
When changing the source in the described-by attributes the screen
reader will read the text of the new source when the status changes.
Just changing the text within the elements holding the descriptions
does not work.
Bug: T291284
Change-Id: I31cc3061cf6c1f699babe41e99e0711f0eb03646
Screen readers will read the collapsed content despite the fact that
it is not visible due to the way it is hidden with CSS. The more button
makes no sense for these users and is probably more confusing than
helpfull.
Bug: T291277
Change-Id: I71888d8b9565d5ee85c5e7a48965e9e9a76eb984
ARIA-selected only works on specific elements/roles. I tried several
combinations here, the most fitting seemed to be the option role, but
that role did not work very well in FF with NVDA. It also should only
be used as direct child to a listbox e.g. with several children.
The next role that's working with ARIA-selected that seemed fitting
is the gridcell. It's still a bit hacky but works well in IE and FF
with NVDA. I suspect that that's pretty good coverage already then.
Bug: T291284
Change-Id: I85c865b0ab12d3923e472e5f36b5c07b7c722180
The following keyboard combinations are now supported on MacOS:
1) CMD + DELETE
2) CMD + DEL
3) CMD + FN + DELETE
(1) for keyboards without a DEL key use DELETE/BACKSPACE
(2) for keyboards with a DEL key
(3) FN + DELETE is a built-in an alias for DEL
Bug: T294519
Change-Id: I896837954b2f0b36a25484080e57d929a5abf774
For example, the Citoid extension adds a "change reference type"
button which should be visible when editing a citation template.
TODO: Decide whether we want to hide the "hide options" button in
this case. This should be handled in a separate patch and possibly
in a follow-up phase, it requires deeper changes to the logic.
Bug: T294351
Change-Id: I1c6c322fe48044d7e726bf20ba7cd2eda422cd8b
I can't reproduce this error, but I feel better having this safe
guard in place. In theory it's possible to trigger resize events
before the TransclusionModel is initialized.
Change-Id: I4bbac0f73873813629ff854ee728465c6e2a4ba7
I6909b3f0b2c153b7ee9995441e995ffa793eab40 was rebased, but
I0226ca7d39e04a69617c0d8a5d3c293cfc9e0709 was merged in the meantime.
Bug: T293202
Change-Id: I3cfec9ebc135eaf998a0982f458cfe75bfb2f01c
We're mainly interested in the layout of a button here. From a
semantic perspective this is just a header to an editable area
and not a button.
Bug: T291284
Change-Id: I683cca2e7d6549e652bd03ae1e46f4eff8c07d0a
This is a more radical change, compared to the previous patch.
I will post more detailled explanations as comments on Gerrit.
Change-Id: I6909b3f0b2c153b7ee9995441e995ffa793eab40
This is done for a lot of the elements in this class. They are trivial
jQuery elements instead of OOUI widgets. While we usually want to use
OOUI widgets, this is different in this case. Think of a template with
1000 parameters.
Bug: T291284
Change-Id: Ie1960ee706dca17aa4963c23a2e89c1cfff106f9
The event "focusTemplateParameterById" targets the right side of the
dialog. The input field for the parameter should be focussed. This
doesn't make any sense when the parameter was just removed (i.e.
unselected).
Change-Id: Ie75b1edaebe9d0444b98e66cb56a5c7774393bea
Preserve the place of annotation meta tags; adds information for the
users about annotation and, if necessary, annotation range extension.
The messages and individual handling of annotations for the annotation
range can be defined by the extensions: see I0b58a418 for an example
of how that can look like.
The structure of this patch closely follows the one from I104e7abbd
(handling of <noinclude> et al.).
Bug: T261181
Change-Id: I39029e4a63d22b37107edec066006557bcff34bf
It should be possible for extensions/skins to trigger the click event
It should be possible to do $('#ca-edit').trigger('click') so that
Vector's sticky header can trigger the VisualEditor.
Bug: T293159
Change-Id: I58625ffca982edfbc4cdcb14a666d79093e1b89d
Removing the selected item causes StackLayout to select (and scroll
to) its first item. To prevent this, we preemptively unselect.
Note that even when an unselected item is removed, StackLayout still
clears the selection, so this patch doesn't lose any useful
behaviors.
Fix should be pushed down into OOUI, unless there's a use case where
we want to select the first item?
Bug: T293635
Change-Id: I0c1fddfa32b76621a9f1328c8173f0158386aee8
The signature of createInvisibleIcon was changed but this
was never updated.
This fixes invisible template renderings inside previews, e.g.
inside the reference context item for:
<ref>{{InvisibleTemplate}} Content</ref>
Change-Id: I3d1b7a177408032957ac3fa8ead813438aa6bda7
Load errors are already handled in the MobileFrontend part of the
mobile visual editor, by this code:
66c55573e5/src/mobile.init/editor.js (L375-L387)
// Wait for the data to load before we show the editor overlay
overlay.getLoadingPromise().then( function () {
...
}, function ( error, apiResponse ) {
// Could not load the editor.
(1) overlayManager.router.back();
if ( error.show ) {
// Probably a blockMessageDrawer returned because the user is blocked.
document.body.appendChild( error.$el[ 0 ] );
error.show();
} else if ( apiResponse ) {
(2) mw.notify( editorOptions.api.getErrorMessage( apiResponse ) );
} else {
mw.notify( mw.msg( 'mobile-frontend-editor-error-loading' ) );
}
} );
Compared to our code:
ve.init.mw.MobileArticleTarget.prototype.loadFail = function ( code, errorDetails ) {
...
(1) this.overlay.onExitClick( $.Event() );
(2) mw.notify( this.extractErrorMessages( errorDetails ) );
};
The lines marked with (1) and (2) do basically the same thing. And
the function parameters "error, apiResponse" and "code, errorDetails"
are actually the same objects, just with confusingly different names.
This causes the popup with error message to appears twice (although it
isn't too obvious, since the two popups appear in the same place, so
only one is visible), and also causes bogus data to be sent in event
logging (T237063).
Bug: T237063
Change-Id: I7fe7a944707fe585251ce9e16bbb78ccd123a7ed
When I type e.g. "subst:example" as the template name, we made this
work as the user would expect: the template named "Example" is found
and it's TemplateData documentation used. But the dialog title shows
"Subst:example". Note the uppercase "S". It means this string is
parsed as a title, including the "subst:". This is confusing. Just
show the template name.
Change-Id: I9817786991a8379cf48b0a664aef1413abddee2d
Some cleanup to improve readability and reduce the amount of code.
Relevant bits:
* One method name was wrong. It can actually return parameter ids,
not only top-level part ids.
* I got rid of some fail-safe checks that are never needed or moved
them to a more central place.
Change-Id: I08f2ad7bc7d3f985d6189dff170dda554f3d37c2
This patch fixes two issues:
* The bottom corner of the new sidebar was never correct because of
a `padding: 2px` that was introduced later, but never compensated
for.
* The moment the toolbar is shown it's not a single-template dialog
any more. This implies minor style changes.
Bug: T290262
Bug: T292727
Change-Id: I08da73880c469085994ee4beb3fcdd973f80ae11
The idea of this piece of code is to make sure both sides of the
dialog show the same element. But it doesn't make sense to force the
*header* of a template into view when I clicked on a *parameter*.
Bug: T292718
Change-Id: I9945f8e54c856152f05bf717e43468ab5ab24d2f
I run into this in some local test. There are two reasons this code
can be reached:
* When a wiki doesn't have the TemplateData extension, the
additional API call from line #154 will fail. But the original
search query succeeded. We have the `originalResponse` and can
return it. This makes the code behave as if the additional
TemplateData API call was never done.
* But what if the original search query failed? We still end in
line #183 – as we should. But this time it can't return anything
but undefined. This will be considered a valid, successful API
response. But it isn't.
There might be a better way to clean up this chain of promises.
This is the smallest fix I found.
Change-Id: I02d3d053156da222ee424382007621f314777015
Instead of using an object mapping namespace ids to if they have
subpages enabled or not, pass an array of the namespaces where
subpages are enabled, reducing the size of the configuration that
gets loaded on all requests. Only requires a minor update to the
JavaScript that uses the value (check for array index instead of
object value).
Bug: T291729
Change-Id: Ia0ecac71721eceed52cc90f39ecc560bdf1b7f9b
I moved some code around and found that quite a lot of code wants to
know:
* Is the length of this transclusion exactly 1?
* I need that 1st part.
There is more that can potentially moved from the dialog to the model.
But I don't want to make this patch to big.
Bug: T292371
Change-Id: Ia94ed0450d04dd97c4c41f5bf7c266f9a534e821
This includes some moving of code. These helper functions seem to make
more sense in the TemplateDialog class.
Bug: T292371
Change-Id: I004405bab60a569b084f9083fefa41f44f9a5561
I hope this code is a little cleaner. This patch doesn't change how
the dialog behaves.
Bug: T292210
Change-Id: I237812b3404437948eb76d8b36dcca2a4c688d6d
This method does not only select a "part", i.e. a top-level item like
a template. It also selects sub-items like template parameters. The
new name reflects this better.
Change-Id: I51a8ddbd05b283248afba5a623cc52da7b2434f5
The TemplateData extension is not limited to the Template: namespace.
And even if pages outside of the Template: namespace typically don't
have TemplateData information, the API is still able to automatically
extract parameter information from every page.
Or:
It's rare that a page outside of the Template: namespace is used as
a template. But if it is, this is not a mistake. The code here in
VisualEditor should not try to be "more clever" than the user is. If
this is what the user want's to do, let's not block them.
Bug: T291883
Change-Id: Iaf3fd5530b74fab7cedfc85ce04c8c40632df11f
The feature set is (almost) fully covered by other tests, notably
the tests for the …OutlinePartWidget base class and the other two
subclasses.
The only bit that's not covered any more is the
"visualeditor-dialog-transclusion-wikitext" message. But that's
super minor and not worth a separate QUnit test.
Bug: T291157
Change-Id: I574f9cff0baf3dff885094769c124a9e05a1d1c8
The code that uses it is commented out
Bug: T291729
Follow-up: I7af2bc91524e832555b66f090a671672cd14f294
Change-Id: I4cceb9ca83a2274fa93783af3608b9486b773522
It's not only used as an event handler, but called as an ordinary
method as well. Let the name reflect this better.
Change-Id: Ie5a0d9c4cd072063a164886f18d0859327b3f267
Note this patch is somewhat incomplete. The feature fully works and
I would like to see this patch merged as it is. But whenever you
press one of the keys the focus is stolen by some element on the
right side of the dialog. This makes it impossible to e.g. press
Ctrl+Shift+Down multiple times. The idea is to work on this in the
next patch.
Bug: T290262
Change-Id: Ic67f2a696c94f1e5c71134d681161221aecbfdf6
Reasserts scroll and highlighting when toggling the sidebar, in the
case that the other panel is hidden (narrow-view mode).
FIXME: Doesn't reassert focus because the page.focus() and
bookletLayout.focus() methods don't seem to work, maybe the bug is
especially prominent when the item to focus was already marked
active.
FIXME: Stopped working for right-to-left sync on wikitext elements.
This is a less common use case and can be addressed in follow-up.
Bug: T290975
Change-Id: I94f5709e810c63ee5fd7729a192ac7b92686b88f
… obviously only to methods that are meant to be private, i.e.
only called from within the class (and possibly tests).
Change-Id: I581558078dc7210abac5f5724f71316ac45745e6
The .onUpdateOutlineControlButtons() method doesn't describe what it
actually does. This issue was introduced in I9c5478a. (Intentionally,
to not make the patch to complicated.) Let's continue to rename
things to be a) unique and b) honest about what they do.
This is an alternative to I8d98e61.
Bug: T289043
Change-Id: I4d52ffa6e9e5df2025a0c33031c1517bcb421279
I can't really tell what insight we get from the word "container".
Every widget is a "container" in some sense, isn't it?
This widget is just _the_ outline, I would argue.
Other suggestions?
Change-Id: I1fb27ee58c1a3dd790022504e978198dadf7ea02
This prevents multiple highlighting, which was possible to achieve by clicking
sequentially in the input fields of two parameters from different templates.
Change-Id: I404936f1569ab544b693a9bc6921381636ea8f40
This is bad for multi-template transclusions, where we focus the
first parameter of each template ending with the last. It's also
inconsistent, we don't do the same for wikitext chunks.
Change-Id: I720ce1a380a6f4a8618c3608b63557df5fb50393
This was damaging the UX by causing the first parameter to be marked
as selected, but without reliably focusing it. For example, loading
a wikitext-template-wikitext multi-part transclusion would cause the
initial focus be given to the documentation link in the template
content header. After this patch, the focus will be at the top of
the page and tab will run down window functions and then through the
sidebar.
Change-Id: I84131870ae3887dcae74d91d68c5984d1dbffd85
Most block-level things that can be interacted with by clicking have a
highlight. Categories don't, and that makes it harder to discover that
you can edit them.
Change-Id: I6be3824c34b36bd09bbae8cab9a9f36b6bcdb767
The weird auto-scroll feature is described in T289043#7297679.
This also fixes T291381 different than I393a2b1. Only one of the
patches should be merged.
Bug: T289043
Bug: T291381
Change-Id: I70d87f12fd68001e880510fb6c38d7c419d64b15
AddPart in the resetDialog also triggers replace part
so this call could be removed there.
Bug: T291365
Change-Id: Id6c0f5bf3aaece45da37ffab75a4d99c113944f6
This is mostly re-arranging existing code. Actual changes made:
* Remove the message that claims a template can't exist. We can't
really know this.
* Instead show the message about "modifiers" in cases where curly
braces and other wikitext syntax is involved.
Bug: T290140
Change-Id: I713d7f54cad2510f9a02c113600980cba8c3e58b
The titles in the link cache do not include subst: anymore so to see
if these pages exist we need to use the same link title used in the
query.
Bug: T290140
Change-Id: I18de81e0bf46212c2199a948f7ca89182aa19eff
This code was introduced in I8fafee6. I can't tell any more what
the "bug" mentioned in the commit message was. Let's get rid of the
duplicate code path, see if we run into regressions and deal with
them one by one. That's much easier to handle than keeping this
confusing code path around.
Note this "focusin" event handler was actually re-implementing
parts of the upstream BookletLayout, namely
OO.ui.BookletLayout.onStackLayoutFocus().
Bug: T289043
Bug: T291381
Change-Id: Ib386ae6efec08465122f0e8ee81cd6dc9a2d337a
I was (again) wondering why the try-catch is there. But it's actually
needed. Now covered by a test.
Bug: T290140
Bug: T291062
Change-Id: Iccc7274ed9e7b81b8491dbca7d3d771706b0ed09
Note there is still an issue with the upstream
OO.ui.BookletLayout.selectFirstSelectablePage() method stealing
the focus in some situations when you press space. Still this patch
already improves the situation. Pressing space on both top-level
template elements as well as parameters should scroll the thing into
view, but keep the focus in the sidebar. This was just not happening
at all.
Make sure to use a very long multi-part template to test this.
Bug: T289043
Change-Id: I9c5478a04b14b94ccd5d00480d48a7d59b4e0c37
We broke something with the change I166b971. When we renamed that
method it started to override (and therefor disable) the method
with the same name from the base class.
I decided to move all code in the subclass for the moment. It might
be misplaced there (note how almost all code related to the new
sidebar is in the base class). But this is cleanup work for later.
Bug: T289043
Bug: T291151
Change-Id: Id255585e78967eee0f72c27727cd23211674923c
The most basic fix. The "outlined" flag is set to false for the
cite dialog, in contrast to the transclusion dialog. It's always
true for the transclusion dialog. Note it doesn't mean the sidebar
is visible, but specifies if a sidebar is created in the first
place.
Bug: T291241
Change-Id: I5a8b538949e9fd0b8e85a6a91ca2420ef72e4612
We removed this line of code in a recent patch, but it turns out
it's still necessary in at least one situation:
* Make sure you have a multi-part template where the first part is
a wikitext snippet.
* Edit the template.
* Click the very first item in the sidebar.
Nothing happens. But the text cursor should be in the wikitext
field.
Another situation:
* Put the text cursor in the first wikitext field.
* Press shift + tab. Now a button in the bottom toolbar should have
the focus.
* Click the 1st element in the sidebar. Again, nothing happens.
The extra .focus() call is redundant in many situations. But it also
doesn't hurt to repeat it. It will just re-focus the element that's
already focused.
Bug: T289043
Change-Id: Iccbe376b98a1b1e5469cd17e1c95d5d8869442d3
It was called like this because the event is literally called "set".
But it doesn't explain _what_ is set.
The new name .onBookletLayoutSetPage() already appears somewhere
else in the codebase.
Change-Id: I166b971c08f5d0fae97fc9d6244117a680f84b7c
The previous patch Id314ee8 was incomplete. The event changed.
The id in the event is not guaranteed to be a top-level partId any
more, but can be a template parameter's id.
Note: "Parameter id" and "pageName" is the same. The fact that
these ids match is how the left and the right side of the dialog
communicate.
Bug: T289043
Bug: T291151
Change-Id: I391f0f8edb96398fd33a2e0b01003013c52776da
I realised these are vital information to make the buttons at
the bottom of the template dialog behave sane. It's still
possible to focus this page, even if it doesn't have a visible
item in any of the old/new sidebars. This is when these flags
are used to decide if the up/down/remove buttons should be
enabled.
Bug: T291151
Change-Id: I6ab709b856d110bfb37daa1592c0b6a99714aa25
The OO.ui.OutlineOptionWidget class does have an .isRemovable()
state. This is how OO.ui.OutlineControlsWidget decides if the
remove button can be used.
It appears like ve.ui.MWParameterPage forgot to mark required
parameters as not removable.
This makes some oddly specific code in ve.ui.MWTransclusionDialog
obsolete. Note how this class does not contain any other code
about "required" or other flags specific to parameters.
Note that one aspect of this patch will be visible in the old
sidebar: The trashcan will be visible on required parameters, but
disabled. It was hidden before. However, this actually improves
the UX. Hiding the trashcan made the up/down buttons jump around.
This makes it unnecessary hard to hit them. It also causes visual
distraction when navigating the list of parameters.
Let's stick to the upstream OOUI behavior.
The remove button still disappears when the only element in the
dialog is the template search widget. This is clearly an
entirely different state. (Don't ask why the up/down buttons are
not hidden. It was like this before.)
Bug: T289039
Change-Id: If78881e503f19f497f1993da4e5b9b09ee538307
The separate setup method was introduced in 2014 via I7c3c133.
It appears like most of the code here was written before this
method existed. Let's update it.
* this.outlineItem is guaranteed to be set. No need for the `if`.
* The parent method is effectively abstract. There is no point in
calling it, I would argue.
* The return value is never used. I.e. this method is never
chained, and probably shouldn't.
Change-Id: Ida26ebdf09be74958936c3950ebdf6def9a69bc0
