This behaviour changed in Ife957374cb0d21446db2067171e68fb726ad8347
and related changes
Bug: T214049
Change-Id: I48df96eedebc6e34d62d1cdb02cddb7a091dae86
The $hasLangauges and $hasVariants checks were used in couple places,
which lead to the same code used in many places.
Following the DRY rule, let's implement a Service that can do that
check, and use that service everywhere in code.
Bug: T224735
Change-Id: I46d58758356e870c408a74b2c087a42d6ad0ddea
Help with readability by using module.exports and require rather than the MobileFrontend
provided mw.mobileFrontend module manager (and avoid adopting webpack at this time)
Replace usages of mw.mobileFrontend.require with local require and module.exports
(compatible with RL or Node implementation)
Changes:
* Notifications modules are merged into skins.minerva.scripts and initialised
via a client side check.
* new file overlayManager for exporting an overlayManager singleton
rather than being hidden inside resources/skins.minerva.scripts/init.js
* All M.define/M.requires swapped out for require where possible
The `define` method is now forbidden in the repo.
Bug: T212944
Change-Id: I44790dd3fc6fe42bb502d79c39c4081c223bf2b1
In I02f8645aac1d7b081eaba8f2ac92a2ef51f78182, Page.js was made into a
model and its html parsing responsibilities were moved to
PageHTMLParser. Additionally, a singleton for the current page
(currentPage.js) and a singleton for the curent page html parser
(currentPageHTMLParser.js) were introduced to replace the usage of
M.getCurrentPage().
This commit refactors Minerva to make use of these changes.
Notable changes:
* 🐛 The event bus singleton was added to references.js since it
previously relied on an instance of Skin to listen for the
`references-loaded` event. However, this event is emitted on the event
bus singleton.
* Additionally, I didn't see a reason why the `references-loaded` event
needed to also pass the current page instance when the only file
listening to it is references.js (which already has the current page
instance) so I removed that and the convoluted passing of page.js within
the file. I assumed this logic was unecessary.
* The call to drawer.showReferences in references.js now was made to
pass the currentPage instance as well as the currentPageHTMLParser. This
is to prepare for I6e858bbe73f83166476b5b2c0fdac1cca7404246 where the
getReferences() interface for ReferencesMobileViewGateway.js and
ReferencesHtmlScraperGateway.js is refactored to accept both of these
instances. Additionally, references.js was refactored to support event
delegation which gets rid of some parsing/setup logic.
Bug: T193077
Depends-On: I02f8645aac1d7b081eaba8f2ac92a2ef51f78182
Change-Id: I2f32dbcc4ebaa4bebb297cda1ecce3457922b343
This test started failing on us for no apparent reason.
Example: Ic95f7b0
https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-docker/11469/consoleFull
Output: "test control group is about 25% (30.8%)"
It appears like the bucketing is not really done based on an actual
random number generator, but based on a hash that contains the session
ID. If this session ID is not really a random number, the hash might
not be random enough as well, but be skewed towards one or the other
direction.
We propose to take the normal distribution into account and change the
narrow +/- 10% margin to +/- 20%.
Change-Id: Ib163f1de4f9cff27aaf8dbc81189315142ff0d8a
The number of generic menu entry specific files is growing which is
cluttering the Menu/ directory. Move the entries to a new subfolder.
Bug: T214540
Change-Id: I807d6f6034ee1924e3a606f5e6782c3298896825
* Introduce a HomeMenuEntry class and use it for adding the home menu link
* Provide override methods for text and CSS class
Bug: T223210
Change-Id: I37160887478cba829a6e2f10a4d8f87d95167556
Two tests are failing. These are false positives that
obscure real failures. Both are removed for the following reason.
1) Editor test is covered by the redirect tests - failing due to
concurrent edits (possibly because multiple browser tests act on the same
page at the same time). However, editing is already covered by the other
2 cases.
2) Remove unwatch test - this is failing as there is a problem in the
setup - the page starts unwatched when it should be watched. The test
for checking that an article can be watched should suffice here.
Bug: T224947
Change-Id: I3049e1b190c3cb2ddc198a45681f59782f770d6a
MediaWiki Core defined $wgHideInterlanguageLinks that can be used to
disable the interwiki links. Minerva skin should respect this config,
furthermore, this config should take precedence over the Minerva's
$wgMinervaAlwaysShowLanguageButton config.
Bug: T214540
Bug: T221792
Change-Id: Id4fe8b67a17f9c28c00a8a3a207946e146502cde
The isAllowedPageAction is used in multiple places (SkinMinerva
and in PageActions toolbar builder). This logic should be defined
in separate service, easy accessible for different parts of the
Minerva skin.
Changes:
- Introduced MinervaPagePermissions as a centralized place to manage
user permissions
- Introduced MinervaNoTitlePermissions, an NullObject pattern to
handle situations when we do not have Title object (like in CLI)
- removed Minerva.ContentHandler service as it's not required any
more
- moved all permission names into constants
- moved isTalkAllowed() into MinervaPermissions
- renamed isAllowedPageAction() it `isAllowed()` to not mix it
with PageActions. Those checks are used in many places, not only
on PageActions menu
- made isAllowed( watch ) more robust - now it checks that Title
is watchable
Bug: T221792
Change-Id: I87d44a9c717b5f752b8d1fd2f146d7f5eef3c53f
Rename SkinMinerva->prepareUserButton() to
prepareUserNotificationsButton(). The function is responsible for
inflating the button that displays notifications when pressed. This name
would be especially confusing when an actual user button is added in
T214540.
Bug: T214540
Change-Id: I8965ef4c5b29ea692d67e821a06131ad5f287287
As a hold-over from a previous porting attempt, thw World.js file
(a cucumber.js convention) exported an instance of MWBot. This
instance was used in several tests, however, since MWBot had been
instantiated multiple times since then, the original edit token
was invalid, causing several tests to fail.
Bug: T224947
Change-Id: I56c06600c43d53bbc4e103d446a1de7a52c2cfad
In some edge cases the RequestContext::getTitle() can return null
instead of Title object. Similar situations already happened in the
past (see T179833). The RequestContext::getTitle() documentation
says it can return null, therefore code should be resilient and
support such situations, even if there are not common.
Bug: T221792
Change-Id: I842f8c49f20e511fda3b081e59a06586810bc748
Ensure SkinUserPageHelper::isUserPage() returns true for user pages
that are IP addresses.
Also adds the page-action menu to all user pages.
Bug: T220114
Change-Id: I3703899bc9ff0042c74260d36f48a388b78b0b6b
If viewmywatchlist|editmywatchlist permisions were set to false for
anonymous user, MinervaSkin would show a watchstar icon that links
to LoginPage, even if user was logged in. Clicking watching action
would cause browser to reload the page without any effect.
Under the hood - system would redirect to login, and then the login
page would redirect user back to the article page because user is
logged in.
MinervaSkin should respect viewmywatchlist|editmywatchlist
permissions. If user do not have access to watchlist, do not show
watch icon.
Bug: T221792
Change-Id: I26a1133a7ccff6a4adcdc72d594d0902bfa8ff79
The Skin::getNewTalks() is already called in SkinTemplate::prepareQuickTemplate.
There is no need to call this function again, as is pretty heavy. Instead of
calling it second time, just re-use the value stored in the QuickTemplate
Change-Id: I0e9491405f4d760278db3a423ee14e8f80720291
The Group shouldn't depend upon concrete MenuEntry definition.
Different Menus can present different MenuElements. Code should
allow easy extensions, not limit only to single MenuEntry
definition.
Changes:
- introduced IMenuEntry interface
- MenuEntry implements IMenuEntry
- removed isJSOnly from logic as it's related only to one menu
element (Watchstar) and Group shouldn't be aware of some special
handling for some elements. The IMenuEntry shouldn't define this
method
- getName, getComponents, getCSSClasses should have defined return
types
Bug: 1221792
Change-Id: I0646df734e869c26bfa8c3a772200e8258a8acce
Additional changes to tests:
* pageExists uses brower.call to avoid token error in talk
* use a before rather than beforeEach for creating articles
to avoid an unnecessary API action
Bug: T222517
Change-Id: I44cda7d62e5e4e58ed38b15ae13fdb0c8dc2e900
This selector was changed in I9ded45388dbcb511cc4bf56859f54318e3e168d9
but the tests are still looking for the old selector which makes the
following tests fail:
- Reference popup drawer.Opening and closing the reference drawer
- Reference popup drawer.Opening a nested reference
Bug: T222475
Change-Id: I4ab8a21146dfd8204151659ff587e77bcadcaf9d
In anticipation of the changes in
I30aa36a0cb89988d1bad8256e5d3f737380bc0a7 which removes the
.wikibase-editor selector from the TalkSectionAddOverlay textarea, the
browser tests need to be changed to a more accomodating selector.
Instead of finding the textarea from the .wikitext-editor selector, the
tests now look for the textarea in the overlay (there is only one). This
also makes it more in line with its method for finding the input element
(.talk-overlay input) as well.
Bug: T221624
Change-Id: Ifbba23e9aee6f68033efc3279ed737d519a7bf41
Per T199939#5096023 use job runner to execute the job runner
as part of the test to ensure the API can return categories for
a page.
Bug: T219920
Change-Id: I467f6635fd0d80a01428f6b06a8c750430d63d6d
Changes:
- moved all menu elements definitions from SkinMinerva into
a separate Definitions.php file
- moved menu building from SkinMinerva into includes/menu/Main
folder
- introduced Builder pattern for easy menu building
Minerva/Menu/Main/Director takes an Minerva/Menu/Main/IBuilder
and builds the menu. The IBuilders use definitions from
Minerva/Menu/Definitions file, so all definitions can be shared
across different menus
- used ServiceWiring file to register MainMenu Director as Service
- left class_alias for old MenuBuilder as some extensions still use it
- The hooks system have to stay like that as some extensions
are using it (BlueSpiceMultiUpload and GrowthExperiments).
- introduced AdvancedMenu builder for the AMC mode
Bug: T216152
Change-Id: I210c3f1fa36bbd2f9108d728b12cbb21ee210354
SkinOptions array was used to determine which options are available
for current session. Once we started extracting things from
SkinMinerva class, we found out that lots of things depend on
SkinOptions.
For example MainMenu/PageActionsMenu depend on skinsOptions var.
We could pass $skin object as dependency to a menu builder, but
this would cause a circural dependency (Skin depends on menu builder,
menu builder depends on skin) which is an anti-pattern.
In order to avoid such situations lets prepare first, and extract
the SkinOptions to a separate class, register it as a service
so different parts of Skin Minerva can freely use a single instance
of SkinOptions object.
Bug: T216152
Bug: T221012
Change-Id: Icd5da546e1bfaf8d9bfe86dab3b659a88eae19e4
SkinMinerva cached the ContentHandler object for better performance.
In the future the ContentHandler will be also used in the Menu,
for better readability, store ContentHandler as Service.
MediaWikiServices will initialize service on first access and cache
it for future needs. Same applies to SkinUserPageHelper,
Bug: T216152
Change-Id: Ia98dc860862360a68556272714669f0c3a13eb1e
Uncover a bug and fix it in the process \o/ - it seems that the
close icon is misplaced between clicking the notifications icon
and loading the contents of the overlay - this confuses the webdriver
as the button is not clickable.
Bug: T219920
Change-Id: Ib4d076fd9b7ea1cd48b6b58940a50560eacd51a0
This migrates the first of the browser tests which had a @login
step from Ruby to Node.js
Bug: T219920
Change-Id: I84e217e2a781aab9eb10e7d873c527d578ec8fd4
On the Selenium daily job, there are 2 domains -
https://en.wikipedia.beta.wmflabs.org
and
https://en.m.wikipedia.beta.wmflabs.org
Currently the cookie gets set on the former, meaning it doesn't work
This should take care of this, while also accounting for running the
browser tests (as we do in the zuul runs) in a single domain
Bug: T219920
Change-Id: I54838fb8aba559c4d72c444968493dff2de9b4f9
Follow up to Iad954405a5ae0608fd5dc90dd5dfa434b3781037
This test now lives in tests/selenium
It should be removed as we don't want to run this test twice.
Change-Id: Idc99ffdf0fedf65d46095a77a0066c5eef95c960
Porting first selenium test from Ruby to Node.js using the
mocha framework. Starting with `category.feature` test.
Tests are placed in a new `tests/selenium` folder with their
own eslint config.
Bug: T190710
Change-Id: Iad954405a5ae0608fd5dc90dd5dfa434b3781037
To help us test special pages prior to moving them on mobile it
would be useful to make AMC the default on desktop
where the special page override does not exist
This is also probably what editors on desktop using the Minerva
skin want out of the skin.
On top of this, add an amc class to the body tag so we can
target styles at AMC and/or non-AMC users
Change-Id: I7f3141bae71181131ae4878fd21fb6ff4322c8ca
These tests fail as the log_in step seems to be broken in the Ruby
Selenium gem. Disabling these makes it possible to merge code at
the cost of lowering test coverage and risk of regressions.
I'm hoping for help from RelEng to get these restored promptly
Bug: T219920
Change-Id: I8e0883e8988293828801be2769db88ac244cb866
Simplifies the tests by making sure they don't need to know
about how OverlayManagers are created.
Change-Id: I38174d1c2d32290d2b1fde4340a85e362e5c102b
Suppress the redlink drawer for User namespace pages. The redlink drawer
prompts the user to create a missing page but this hinders the usual
workflow for User page visits specifically. A User page is connection to
an account's contributions, age, and other activities and encouraging
the creation of a missing User page when trying to view these
connections is a hindrance, especially if the missing User page is not
associated with the current user.
Bug: T201339
Change-Id: I784493a8ecf28176b5a393cb52d7bfa9fa9b1309
The talk overlay is created inside MobileFrontend, but the
overlay for creating a new talk overlay is here.
The two need to speak to either other - in particularly, the create
talk overlay must invalidate the current talk page before returning
the user to the former.
In preparation for the refactoring changes in MobileFrontend, the
same object is shared and the cache invalidation is moved here since
Minerva creates and manages the PageGateway instance that is given
to the talk overlay.
Additional change:
* Update a selector broken by changes in
I8c34646b7ba13a26facbb69684e65109870d27a1
Bug: T217102
Change-Id: I212ff044c4c608c6ea60a5fda043166cd434ec1f
The top level `nav` CSS selector (combined with the nested element
selectors, i.e. `nav ul li a`) represent a general DOM structure
which is not limited to the navigation menu and can interfere with
other styles.
This replaces the `nav` selectors (which have only been used to select
the main navigation) with a `#mw-mf-page-left` selector instead, since
that represents the main navigation as well.
Change-Id: I047108974fd295f196d9f7150c3721c05ac40c6d
The talk overlay must subscribe to the creation of new topics
so that the list of topics in the talk overlay contains the
newly created topic. It does this by subscribing to the
talk-discussion-added event and forcing a route refresh when that
has completed.
Additional changes to browser tests:
1) QA: CSS selector changed for talk overlay
Since I42fd7b08c4b9d92dee549d06de8a0012ea037d28 the '.add' class
was removed from the talk button. This makes the browser test fail
but is a false positive.
2) One of the browser tests was using the same selector to mean
two different elements - the add discussion button in the talk overlay
is now clearly distinguish from the "add discussion" button that is blue
and appears at the bottom of talk pages
Change-Id: I935b3c5f37baf242c06585ae0e2f13d059b9c324
"cleanuptemplates" was the old page issues module name. The new name is
"pageIssues". Update the test module name.
Change-Id: Ie31e4d1548918463f6e33429ded3abc4bfb08dff
Remove getAllIssuesSections(). This is no longer in use and does not
appear to be sufficiently general purpose to want to maintain.
Bug: T212371
Change-Id: I7ed73408705cba64b26dd318e78ae415b707e687
- Move page issue view components that do not modify the DOM during
during construction to PageIssueLearnMoreLink.js and PageIssueLink.js.
PascalCase is used optimistically for filenaming in the hopes that
these functions can become something like a JSX component. A "new"
function prefix is used in the meantime.
- Move page issue view logic that munges the existing DOM to
pageIssueFormatter.js. Substitute "create" prefixes for insert so that
clients won't forget that calling the function is a modify operation.
Alternative naming welcome but it shouldn't be confused with more
idealistic components that do not depend on DOM state for
construction.
- Consolidate createPageIssueBanner() and
createPageIssueBannerMultiple() into insertPageIssueBanner() as the
code was quite similar and were it a true component, it would probably
be a single component.
All new files appear under page/ to keep their distinction from the
overlay code clear.
Some view logic remains in pageIssues.js but it shall be difficult to
isolate.
Bug: T212376
Change-Id: Iccce709c34fa8de5a28a5a00098add5775e3dc9a
Replace QUnit deepEqual() assertions with propEqual(). The former is a
recursive == check, the latter is a recursive === check which seems
preferable.
find tests -name \*.js|
xargs -rd\\n sed -ri 's%deepEqual%propEqual%g'
Change-Id: I977244d24c47072cc62b7d9fc797505a5f39aa54
Replace all occurrences of `M.require( 'mobile.startup/pathToModule' )`
with `M.require( 'mobile.startup' ).pathToModule`. Where multiple
requires existed, add an intermediate variable,
`var mobile = M.require( 'mobile.startup' )`, and dot off that.
This changes improves the consistency of MinervaNeue which currently
contains a mix of require styles and eliminates any deprecated requires.
Bug: T208915
Change-Id: If14f280672d914d07275197100b12421bb217b67
There is no longer a need to pass in a jQuery dependency.
Don't use the stateful skin to query image placeholders. Use the lazy
image loader instead.
Depends-On: I3d023b3d96bf278666abb956142e5cee12b68b1f
Bug: T214658
Change-Id: I2bf42366c0e27462c32162124d07761b91d66166
* Add inline exceptions for global selectors
* Exception for $.animate in scrolling code
* Fix $ prefixing in search.js
* Fix repeated selectors in search.js & toc.js
* Disable no-parse-html-literal in tests
Change-Id: Id58fe11d1e09714501a378e4ca7ed9588f02f32d
* The check for whether the page issues code has been loaded should
be run just before clicking the banner. It's in the wrong place.
* Now page issues is live across all wikis, no need for "in beta"
check
* Seledctor can be simplified now no need to worry about beta
Change-Id: Ie24a9d9fc1966ca5db2cd0a6a37c1aa6d719924a
Update the API usage for lazily loaded images. This is still clumsy and
may be further revised in future patches.
Bug: T211724
Depends-On: Ic73f78825eaab561e8ed694aa6cc102ccb471f95
Change-Id: Ia708cda688e6bdb12074d85d98f7e98fdf7b0ca8
Clicking the page issue banner (which is ready from first paint to
be clicked) will not yield the overlay. The user must wait for the JS
to load. Likely the reason for Minerva browser test failures against
beta cluster.
Change-Id: I06c488ca64dd44ad24368a1d6b47bb2646ad4552
The test is failing in stable. Since page issues is going to production
next week this patch can be reverted as soon as that has happened
Change-Id: Idd8de17883006e3cc5f5615781a54a4072a78087
The 2 selector approach is flawed as the .ambox element matches a
hidden element in the old treatment which is not clickable.
I suspect this change will fix the failing browser test on the beta
cluster. Integration tests will continue to test the new selector
in the mean time.
Change-Id: I44a873b2e89069c4a47a428c528592159520568c
Improve the comments and APIs provided by AB.js:
- Control becomes unsampled.
- A becomes control.
- B becomes treatment.
This code does not appear to be in use presently, so it's a great time
to change it.
Change-Id: I31d619f889ee45102a4aed774a6ec41f0d95ba7d
This experiments with making PageIssuesOverlay an Overlay with
various options.
The appending of children is a little messy and points at a need
to standardise this some way
(see https://phabricator.wikimedia.org/T209647)
TODO:
* Remove the iconString property on PageIssueSummary which is no longer
needed
Bug: T209647
Change-Id: Iadd798a820dca6bbb31edc9a8570b6db7aac237a
The DownloadIcon is reduced to a factory function that
returns an instance of Icon
Depends-On: I4d703eef68d51bbe0b03579c5cca0845e17b8c9d
Depends-On: I4a4129b2cac7c7c49559beef0b8780f3211edf9c
Bug: T205592
Change-Id: Ib87390d17bef6f50842f52cd84c9ce2b162aaff0
This patch removes the remaining usages of M.on/M.off/M.emit
(functionality derived from moduleLoader.js in MobileFrontend) in
Minerva and continues the work of
Id990b0e1a53221d5c1cb3e3012aed0e27d801fc2 (patch for MobileFrontend).
This patch and the patch for MobileFrontend should be merged together as
they both depend on eachother.
Depends-On: Id990b0e1a53221d5c1cb3e3012aed0e27d801fc2
Bug: T156186
Change-Id: I005d2fcdbf91c2f1ac98178dfa388aa8174e7530
* On commit it needs admin rights which it doesn't have
* On browser tests the toast flakes too much.
Bug: T208808
Change-Id: I1fa93c8f451f3f839030fa7a144b1cb285c4239d
Given the fade in/out animation of the toasts and the instability of
the beta cluster and the round trips to Sauce labs, we're seeing lots
of false positives on our browser test reports
Running these per commit should ensure we see minimal regressions and
get some protection. We'll continue to test other scenarios which do not involve
the toast on beta cluster.
Provided we can rely on green browser tests, we will trust the browser
tests more and they will be more useful than they currently are.
Bug: T208808
Change-Id: Idc601ad462de36f2d6d52fe951194b429e6f824f
We use lots of write operations in Minerva browser tests. On the beta
cluster many of these are redundant, as the page already has the content
required or the page already exists
Limit where we do our creation... the less write operations we make the
more stable we can expect these browser tests to be.
Change-Id: If88b878e14bf4a0424fcf23213653cfc2cf8d87b
* features/search_loggedin.feature
They run on integration and are not super-business-critical.
Given their flakiness and Cirrus's likelihood of being down,
don't run it on beta cluster
* tests/browser/features/language.feature
Tag is superfulous, all scenarios repeat it.
* tests/browser/features/toggling.feature
ocassionally fails on firefox. Limit to Chrome and integration
tests
* tests/browser/features/search.feature
Limit the tests that run against beta cluster to search for
partial text and clicking a search result, since these are
two things we want to check integration well with other extensions.
The rest being testing on the more stable @integration tests
should be more than enough.
Change-Id: Ia2e8d3726212fee30725fdb9167ea38aa41eacbf
The page issues browser test is now compatible with both the old
and new treatments.
For consistency, in integration tests it will be run always in
treatment B since that will be the new way to do this going forward.
The beta cluster will test issue treatment A while it continues to be
the default. So with this we enjoy the best of both worlds.
When we enable page issues, the beta cluster will inherit the config
from production and we can remove the treatment testing for treatment B.
A follow up patch, might add testing for a page issue on talk pages,
which will retain treatment A for completeness if this is deemed useful.
Bug: T206647
Change-Id: I586523e452a6809e310f65a2ed55c6771d1965b6
