Commit graph

208 commits

Author SHA1 Message Date
jenkins-bot 76897f3b1d Merge "Use core table of contents in Minerva" 2019-09-03 21:03:32 +00:00
jdlrobson a93656e491 Reflect terser labers
Follow up to I3cac0e231e254755dc2618e1c2d3b4aef2758c67 which broke
these browser tests

Bug: T226562
Change-Id: Ie2709efe8e3d588d506c2a16e807c517621e4fbb
2019-09-03 19:08:53 +00:00
jdlrobson 15323b3d63 Use core table of contents in Minerva
No more using the TableOfContents component in MobileFrontend. It's
just creating more work for us. The end result is exactly the same - we can make
a table of contents using the checkbox hack rule and CSS that looks identical to
the current table of contents.

For now, this change can only be tested on Minerva desktop.
I2ea1c23bc86871e2a095c4c6674a08ff2f04b160
is the patch that goes for the jugular and applies this to mobile Minerva. It's
important we merge the two together to avoid disruption to this feature, as
currently MobileFrontend strips the table of contents from core from the HTML
using the MobileFormatter.

Change-Id: I720e62a578f0c7a14f4b5a698004471c85e54bc8
2019-09-03 19:02:31 +00:00
jenkins-bot a75049f4b4 Merge "Track all menu interactions" 2019-08-27 10:01:47 +00:00
Piotr Miazga c5f38e466d Track all menu interactions
Changes:
 - added support of event-data-name to toggle list and all menu
 entries
 - track main menu open actions
 - prefix all menu interactions with `menu.`
 - prefix menu opening with `ui.`
 - track tab clicks (also a part of new ui)
 - track notification icon clicks

We're not tracking the Download icon as it has it's own
instrumentation.

Bug: T220016
Change-Id: I442103c1f8967c6710429329f024f266c9b11ea6
2019-08-26 11:43:16 -07:00
jdlrobson 9150aec131 Feature flag overhaul
Two new feature flags:
1) MinervaPersonalMenu
2) MinervaAdvancedMainMenu

Changes:
* AMC defaults to false on desktop - desktop doesn't have AMC mode it just
enables several skin options.
* WHen inserting a link at the bottom of the page check whether the talk at top
of the page (tabs) is enabled.. not AMC
* Update ServiceWiring to construct menu based on MinervaAdvancedMainMenu
and  MinervaPersonalMenu - note when former is enabled but not latter there is
no way to logout. Noted in README.
* Use one entry point for skins.minerva.amc.styles/index.less
* Document files inside skins.minerva.amc.styles to make it clear which features
they are associated with
* Drop history page styles when AMC is disabled - it's not possible to ever get to
these as the history page redirects in non-AMC mode
* Rename the class .minerva--amc-enabled to minerva--history-page-action-enabled
to reflect its real purpose and move styles from skins.minerva.base.styles to skins.minerva.amc.styles
No need to worry about cached HTML as AMC runs without cache...
* Remove isAnyAMCOptionEnabled - it's an antipattern and should be discouraged as it discourages the
art of feature flagging. Nothing is using it after these changes.
* The AMC_MODE flag is disabled. There is no need for this - AMC is not a feature and therefore not a
skin option. It is a mechanism for turning on other skin options. Tests are updated.

Testing:
It should now be possible to enable any feature in `beta` and see it in the beta of the
site.

Bug: T229295
Change-Id: I48959905f5c09721b14a27aa1a5ad82849ac6263
2019-08-23 10:48:14 -07:00
Stephen Niedzielski bfdfc1165c [UI] [menu] slide the main menu over the page
Slide the main menu over the page instead of sliding the page over the
menu. Also, use viewport units for the main and notification menus.

Note, this lays foundation work for T225213.

Bug: T206354
Change-Id: I14b67d1e97b84086ea13e28df8148824a1f493e3
2019-08-12 18:22:45 +00:00
jdlrobson 9aae174a15 AMC features should default to true in desktop
It is expected that in mobile mode, onRequestContextCreateSkinMobile
hook will explicitly set skin options based on the user configuration.

The desktop should however reflect everything with AMC enabled.

Bug: T229295
Change-Id: Ib3bf09c25d9bcf9b31d994b92a1d016cca8a6224
2019-08-01 18:04:00 -07:00
jenkins-bot 721e63f19f Merge "Hygiene: Remove OPTIONS_ and OPTION_ prefixes from SKinOptions" 2019-08-01 19:55:08 +00:00
Piotr Miazga 4efd72a879 Hygiene: Remove OPTIONS_ and OPTION_ prefixes from SKinOptions
Additionally, renamed couple options for better readability.

Bug: T221012
Change-Id: Ia347a60d469fba8f35afa7c70aa806a46271dccd
2019-08-01 10:34:03 -07:00
jdlrobson a0e5139e40 Don't show download button on missing pages
Additional: Pass in a page rather than a skin - skin is not
used anywhere other than to get the page

Bug: T211775
Change-Id: Ia7c56158773ac16992fb1ebf002131e9c24dda14
2019-08-01 16:11:57 +00:00
Piotr Miazga 9ece509c99 Hygiene: SkinOptions should validate options
Changes:
 - when trying to set unknown option, throw exception
 - added unit tests

Bug: T221012
Change-Id: I0d1a43fb5a7179b8e9cd578e3e33bab12ddfd61c
2019-07-31 22:24:29 +00:00
jdlrobson 0311e8ed1c Disable diff test due to issue with RunJobs
Bug: T229031
Change-Id: I9bf37b796ba9ddafdc3be559e74d2b511c3e78b1
2019-07-25 18:38:05 +00:00
jdlrobson f0503a52d0 Refresh the user, bell and search icon
* New module skins.minerva.icons.wikimedia provides icons
from Wikimedia UI in Minerva
* search selector in skins.minerva.content.styles.images is
retained for cached HTML
* skins.minerva.icons.loggedin now a
ResourceLoaderOOUIIconPackModule and bell removed from repo.
* userAvatar replaced with userAvatarOutline

Bug: T224070
Change-Id: Ibed609371060acc4b69e5cd4cd4f20edc871b3ba
2019-07-25 16:35:01 +00:00
jdlrobson 28c84d7d0d Update reference test to click the right element
This behaviour changed in Ife957374cb0d21446db2067171e68fb726ad8347
and related changes

Bug: T214049
Change-Id: I48df96eedebc6e34d62d1cdb02cddb7a091dae86
2019-07-18 14:15:55 -07:00
jenkins-bot 6076c61772 Merge "Embrace packageFiles" 2019-07-16 21:21:42 +00:00
Piotr Miazga d7f60a26db Extract language/variants check to a service
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
2019-07-16 19:53:57 +00:00
jdlrobson 72df451bd3 Embrace packageFiles
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
2019-07-16 18:04:10 +00:00
Nicholas Ray 331df226f5 Make Minerva use new PageHTMLParser.js and refactored Page.js
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
2019-07-15 16:43:51 -06:00
jdlrobson 0fa35053c3 QA: Update search browser test selector
Change-Id: Ibef220e65a4dfa3725ac01d4917fe7a0a9b8a878
2019-07-11 17:26:38 -07:00
Kosta Harlan 14493b00dd Move unit test into "unit" directory
See also I16691fc8ac063705ba0c2bc63b96c4534ca8660b

Change-Id: I90921679518ee95fe393f8b1bbd9134daf0ba032
2019-07-09 10:14:36 -04:00
jenkins-bot 61784701e2 Merge "Make failing Bucketing test more robust" 2019-07-08 15:59:15 +00:00
Thiemo Kreuz 303a5019fc Make failing Bucketing test more robust
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
2019-07-05 21:38:58 +00:00
Stephen Niedzielski 4c34cce9f3 Hygiene: move menu entries to subdirectory
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
2019-07-05 21:32:44 +00:00
Kosta Harlan 9b7b10bbe9 Allow overriding text and CSS class for home menu entry
* 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
2019-07-03 16:42:47 -04:00
jenkins-bot 200e64d32a Merge "Make SkinOptionsTest extend unit test case" 2019-07-02 18:34:09 +00:00
Kosta Harlan b6682f6f93 Make SkinOptionsTest extend unit test case
Depends-On: I237a9f82e4d1b05cf2f08b3e4bb7ffcd8d47111c
Change-Id: I42ca6c315584ae04289b28efa766bf4e67fda67a
2019-07-02 13:54:11 +00:00
jdlrobson a9533fc463 Browser: Drop failing tests
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
2019-07-01 18:02:54 -07:00
Piotr Miazga bb0071383a MinervaPermissions must respect $wgHideInterlanguageLinks config
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
2019-06-21 18:59:50 +00:00
Piotr Miazga 6352190684 Hygiene: Extract isAllowedPageAction into MinervaPagePermissions
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
2019-06-21 15:30:11 +00:00
Stephen Niedzielski 1e61706e0e Hygiene: rename "secondary button" to "user notifications"
The secondary button is not generic. Rename it to match its specific
purpose.

Bug: T214540
Change-Id: I50b7e7fa7bea91a029a8ca71199b0309dff1c518
2019-06-19 11:31:16 -06:00
Stephen Niedzielski 02726555e0 Hygiene: rename user notification build method
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
2019-06-19 11:16:23 -06:00
jenkins-bot 8b32918900 Merge "Revert "temporarily disabling failing browser tests for language selector"" 2019-06-13 18:23:53 +00:00
Niedzielski cb0460aaf5 Revert "temporarily disabling failing browser tests for language selector"
This reverts commit c0b4911670. These
tests seem to be running now.

Bug: T224605
Change-Id: If052e75376ea85d86d395b481f204d3d3049fb3f
2019-06-13 15:24:37 +00:00
jenkins-bot c99188376f Merge "Add method for getting menu entry by name" 2019-06-06 21:46:26 +00:00
Kosta Harlan b035d76d0f Add method for getting menu entry by name
* Also fix typo with $returnToQuery
* More usage of DomainException

Bug: T222834
Change-Id: I5623741779eebe4c5694bdbd7a1aa1bcdd7f04c6
2019-06-06 13:04:29 -04:00
Jan Drewniak 018d436bfe Ensure each api call creates a new instance of MWBot
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
2019-06-06 13:33:54 +02:00
Piotr Miazga 047618c24d Hygiene: SkinUserPageHelper should support Title = null
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
2019-06-04 17:17:58 +02:00
Jan Drewniak 7af32bc309 Treat anonymouse user IP pages as UserPages
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
2019-06-04 11:53:07 +02:00
jenkins-bot 3598618390 Merge "Hygiene: test method names should be written in cameCase" 2019-06-03 17:37:46 +00:00
jenkins-bot 89b9d58b93 Merge "temporarily disabling failing browser tests for language selector" 2019-06-03 16:55:26 +00:00
Piotr Miazga b3a7047d65 Hygiene: test method names should be written in cameCase
Change-Id: I12bd086c0ebd42133b6f7171847dbe50f8f7fe30
2019-06-03 18:53:40 +02:00
jenkins-bot 4ef947da28 Merge "Watchstar should respect viewmywatchlist|editmywatchlist permissions" 2019-06-03 16:16:58 +00:00
Jan Drewniak c0b4911670 temporarily disabling failing browser tests for language selector
Bug: T224605
Change-Id: I4dbc3c5355f795adfb162b4a1a649440e0f36ca0
2019-06-03 12:41:15 +02:00
Piotr Miazga 4ec9b06a77 Watchstar should respect viewmywatchlist|editmywatchlist permissions
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
2019-05-31 17:34:35 +00:00
jdlrobson 94c0926614 QA: Don't chain things that cannot be chained
Causing builds to fail on
https://integration.wikimedia.org/ci/view/Reading-Web/job/selenium-daily-beta-Minerva/58/console

Bug: T223676
Change-Id: Ia285f8bd2f61e6e59aa38cd1909b450328eb053c
2019-05-28 15:43:39 +02:00
jenkins-bot c4b31067ab Merge "Provide IMenuEntry interface" 2019-05-17 13:12:09 +00:00
jenkins-bot 7a83809f4d Merge "Minerva shouldn't call Skin::getNewTalks() twice" 2019-05-14 15:34:44 +00:00
Piotr Miazga 39a2078472 Minerva shouldn't call Skin::getNewTalks() twice
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
2019-05-14 16:09:18 +02:00
Piotr Miazga 6905b85d67 Provide IMenuEntry interface
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
2019-05-14 13:12:37 +02:00