Commit graph

721 commits

Author SHA1 Message Date
Jan Drewniak 1e946a379d Custom page preview for disambiguation pages
Creating a different page preview for disambiguation pages.

This patch:
- modifies the Preview model to accept a new 'type' property
- modifies the Restbase Gateway to pass the 'type' prop to the Preview model
- creates a new template to accept both generic/disambig previews
- modifies the renderer to render the new template
- generates icons for new template through resource loader
- adds new i18n strings
- modifies event-logging "preview seen" event to send new "disambiguation" previewType
- updates event logging schema version
- adds tests for Preview model and renderer for new preview type
- does way too much? yes, yes it does.

Bug: T168392
Change-Id: Idc936cc3eabbdd99a3d98f43c66b4cdbb7d24917
2018-03-14 11:24:26 -07:00
jdlrobson deaaf0961b Remove popups from critical rendering path
Instead load it via mw.loader.using

We retain the module name ext.popups as this will be present
in cached HTML, however now it will load the bulk of the code
inside ext.popups.main

Bug: T176211
Change-Id: Ibe212721807d3698dc45ef46b2dbde15ca9d2f70
2018-03-13 08:44:31 -07:00
jenkins-bot e621fd196d Merge "Give summary a min-height to avoid overlap with gradient." 2018-03-09 19:38:55 +00:00
jdlrobson 3cd45bca2a Give summary a min-height to avoid overlap with gradient.
Bug: T188581
Change-Id: I5b6012b61577ead60dc0d0e87ebcad5042304104
2018-03-09 09:07:19 -08:00
jdlrobson f5f21a8d09 RESTBase url is configurable
Allow developers to use different endpoints for summaries
= developer happiness

This is useful for the following use cases:
* A developer wants to test against a production endpoint via
CORS
* A developer has setup an API where REST is hosted elsewhere
e.g. http://localhost:6927/en.wikipedia.org/v1/
* A user wants to create their own REST summary compatible
endpoint
* A wiki e.g. wikidata wants to use a different endpoint which is compatible
with the summary endpoint.

We are unlikely to use it ourselves on Wikimedia wikis (the
default should suffice) but this will be a powerful tool for

When not configured this will continue to work as per normal

Change-Id: I8a7e12fbc43cddbac678e0d7b81d1e877b747b22
2018-03-09 16:15:19 +00:00
jdlrobson ca440dfbed Remove client side formatters in Popups code base for MW API.
Stripping parentheticals were designed specifically for working around
issues with content inside wikimedia wikis and error prone.

This problem for wikimedia wikis is solved by the mobile content
service.

Given we have no intentions to use the MediaWiki API for summaries.
They are not necessarily useful to third parties and it makes little
sense to maintain them (a third party can configure their own API or
use their own REST endpoint if they really do need them).

Bug: T189042
Change-Id: I2729dc9f172af0afee1c6f0cd563c556b4ae0aeb
2018-03-08 21:05:49 +00:00
jdlrobson 8e47b54796 Remove client side formatters in the REST formatter
These are now taken care of by the Mobile-Content-Service's
summary  endpoint and no longer needed here!

They are actually breaking certain previews so time for these
to go!

Bug: T183833
Change-Id: Icd2a21127c2f5881943564eca4df6bed3c15e223
2018-03-02 12:04:08 -08:00
Sam Smith 58ae3d01ea i13n: Clarify nature of timestamp property
Specifically, clarify why the timestamp property is merged into the data
bag that represents the event when it is.

Change-Id: I184de734e66490fc728a4f9c2f84bf0765aeed08
2018-02-28 18:06:14 +00:00
jdlrobson 535149c16c Upgrade schema and log the required fields
This change updates the schema and begins to log
additional information such as source_namespace, id
and title. This information is provided inside the
boot action.

Additional changes:
* Allow camel case in bundle artifact

Bug: T184793
Bug: T186728
Change-Id: I425ffecc018bef2958d0dfe957a40a065e3e6c56
2018-02-26 10:25:30 -08:00
jdlrobson fe654d7f5e Hygiene: namespaceID => namespaceId
For consistency with pageId and sessionId let's rename
namespaceID to namespaceId

Change-Id: Id0f6f6434691b62d3c5c7ae941970b79e0a99366
2018-02-26 10:23:38 -08:00
Stephen Niedzielski e406d4556f Fix: don't assume thumbnail URLs contain pixel size
Don't assume that thumbnail URLs contain a dimension delimiter of "px-".
Previously, thumbnail URLs always contained the width. e.g.:

  https://upload.wikimedia.org/wikipedia/commons/a/aa/100px-Red_Giant_Earth_warm.jpg

However, thumbnail URLs that actually point to the original are not
sizable:

  https://upload.wikimedia.org/wikipedia/commons/a/aa/Red_Giant_Earth_warm.jpg

These are provided, for example, when the thumbnail size requested is
larger than the original. There was code designed to handle this
scenario but it only applies when RESTBase and page preview thumbnail
sizes happen to be in sync. In other words, if RESTBase requests a large
thumbnail on behalf of page previews, and page previews only requested a
small thumbnail, the original may be unexpectedly provided. A
conditional is introduced in this patch to verify that "px-" is actually
detected. If it is not present, the original is used.

Bug: T187955
Change-Id: If4e29dd870aecd6d461cc8203f6576d1bb8844f2
2018-02-22 12:36:30 -06:00
Sam Smith 35daa2a689 Hygiene: Page view -> Pageview
Pageview is consistent with verbiage used by Research and Analytics
Engineering in their reports and documentation, e.g.
https://wikitech.wikimedia.org/wiki/Analytics/Pageviews.

Bug: T184793
Change-Id: I8ae085b4af85aa72f234f3db27f0cac2c4d014e5
2018-02-21 18:51:49 +00:00
jdlrobson 21f2d4ab0f Model should capture page id
We're going to need this for logging page views.

Change-Id: I1fc46a9fab54f512edaf5feb5abbaa4f025dcb4a
2018-02-19 16:46:13 +00:00
jdlrobson a702c0f499 Capture page view-like interactions
* New action added PREVIEW_SEEN
* The action will be used to signal that a page view needs
to be recorded.
* PREVIEW_SEEN is a delayed action which is triggered
as a side-effect of the previewShow action. It is only dispatched
if the user is still previewing the same card and the page
related to the card has preview type `page`
* The pageview changelistener is added when
$wgPopupsVirtualPageViews is set to true.
* The page view changelistener listens for page views and logs
them using EventLogging when needed using
https://meta.wikimedia.org/wiki/Schema:VirtualPageView

Note:
* Currently if a user has enabled the DNT header, the
event will not be logged. There is ongoing discussion on the
ticket and fixing this will be addressed separately.
* Only title and referrer are logged in the initial version.
The task demands that "namespace" is logged but this information
is not provided by the summary endpoints we use so will need
to be added later (if indeed needed) either via a change to that
endpoint of by using JavaScript to parse the URL.

Bug: T184793
Change-Id: Id1fe34e4bdada3a41f0d888a753af366d4756590
2018-02-16 23:03:33 +00:00
joakin 1fff0d2ea7 Improve & fix action and integration tests
This change fixes some issues with assertions not running, removes
unnecessary promise dances, and improves legibility and some code
patterns in the action and integration node tests mainly.

Detailed changes:

* actions.js
  * Fully migrate out of jQuery 1 promises (no done/fail)
  * Fix linkDwell action not returning the fetch action promise

* actions.test.js
  * Simplify setupWait for the tests
    * It always autoresolves immediately the wait call to ensure speedy tests
    * No waitDeferreds or waitPromises array coordination, rely on action
      returned promises instead
  * Get rid of that = this in favor of arrow functions
  * Rename generic "p" promises to meaningful names
  * Add assert.expect for more solid tests (so that we don't skip assertions in
    the future if we change them)
  * Fix some assertions that weren't being run because of the incorrect promise
    being returned (p.then, and then just returning p)
  * Get rid of $.when stubbing in favor of waiting for the promise returned from
    the action
    * Get rid of hacky setTimeout(..., 0) to run assertions after the promises

* integration.test.js
  * Get rid of wait(0) calls to hack around asynchronous actions
    * Use the action returned promises instead of the waitPromises/Deferreds
  * Remove unused "el" parameter being passed to this.abandon in several tests
  * Remove unnecessary test helper this.abandonPreview (it was the same as
    this.abandon)
  * Clarify a bit the last and more complex test with some comments and variable
    name changes
  * Get rid of that=this in favor of arrow functions

* container.test.js
  * Get rid of that=this in favor of arrow functions

* previewBehavior.test.js
  * Get rid of that=this in favor of arrow functions
  * Get rid of $.each in favor of .forEach

Bug: T170807
Change-Id: I06fafd92a1712f143018e2ddff24fadf1a6882b3
2018-02-16 13:52:56 +01:00
joakin 2df22ef2b2 Don't leak deferreds out of functions
Always try to return a promise.

Bug: T170807
Change-Id: I4a48b6e40a5398a743e39589b5c2fcac4482a814
2018-02-15 15:01:36 +01:00
Jan Drewniak 95b880aa29 Return promises from action thunks
Returning promises from the `linkDwell` and `abandon` thunks and
removing some of the `wait` stubs in the unit test for these actions.

Also converting a fetch callback from a `.fail` to a Promise A+
compatible `.catch`.

Bug: T170807
Change-Id: I4bbf2863db090e222ba926d3bc36a99da4bdb601
2018-02-14 20:08:05 +00:00
Jan Drewniak 10465c8bf8 Centering settings dialog and overlay
- Removing the javascript positioning of the settings dialog.
- Placing the settings dialog inside the settings overlay.
- Using flexbox to center the settings dialog.

Bug: T157072
Change-Id: If8d929fe019a04ed5f96aa593779841a52f58eff
2018-02-14 17:54:38 +01:00
Ed Sanders 998d7c4f22 build: Update linters
Change-Id: I1af64c55fdd26a2923a1ec20500e97ea13d92bc8
2018-02-04 22:09:45 +00:00
joakin 807100bcca Limit line length to 80 characters
Enforce it with eslint.

Ignore:
* Comment lines with eslint disable directives
* QUnit test lines as they contain long subjects (QUnit.* (only, test,
  module, skip, etc)
* Strings, since long strings are used extensively in tests
  * Ignore template literals for similar reasons
* Regex literals as they may be too long, but can't be easily
  split in several lines
* Long urls

See bug for more general proposal for eslint-wikimedia-config.

Bug: T185295
Change-Id: I3aacaf46e61a4d96547c513073e179ef997deb09
2018-01-19 14:20:39 +01:00
joakin 7169210406 Remove unnecessary .promise() call
In the mediawiki gateway fetch uses mw.Api which when calling ajax
returns a promise (not a deferred).

Thus .promise() here is unnecessary and happens to work because of
jQuery promises but it is not a standard method on JS promises so it
shouldn't be used on promises, only on deferreds.

Change-Id: Iec609b90bffad8b99b3908897dfb72d7c4ed5481
2018-01-18 18:07:43 +01:00
Stephen Niedzielski 9114981380 Update: show placeholder preview for more failures
Functional changes

- Show the default / error preview for all extract request failures
  except those due to network circumstances (such as CORS) or no
  connectivity (offline). Previously, the error preview was displayed
  only for missing pages.

- FETCH_COMPLETE was previously only dispatched after FETCH_END. Now
  it's also dispatched after FETCH_FAILED. The additional "fetch
  complete" is not expected to impact logging. The states of success
  are: START, END, COMPLETE. The new failure states are consistent with
  success: START, FAILED, COMPLETE.

Testing

Errors may be stimulated in a number of ways including:

- Timeout: add a timeout field to RESTBaseGateway /
  MediaWikiGateway.fetch().

  http://api.jquery.com/jquery.ajax/

- Bad request: change MediaWikiGateway.fetch's action field to
  `Math.random() > 0.5 ? 'query' : 'fail'` and RESTBaseGateway.fetch's
  url field to
  `RESTBASE_ENDPOINT + ( Math.random() > 0.5 ? encodeURIComponent( title ) : '%%%' )`.

- Desired Gateway can be configured in Gateway#createGateway().

- Note: T184534 describes a circumstance where cached previews may not
  appear when offline. Disable browser caching to avoid confusion.

Bug: T183151
Bug: T184534
Change-Id: I7332284da0e0fb1ecd234a6f1e146ebd9ad8d81f
2018-01-16 18:44:00 -06:00
Stephen Niedzielski 6ba57786fb Update: missing preview copy
Update the placeholder extract and button text shown when a page preview
is unavailable from:

  "popups-preview-no-preview": "Looks like there isn't a preview for this page"
  "popups-preview-footer-read": "Read"

To:

  "popups-preview-no-preview": "There was an issue displaying this preview"
  "popups-preview-footer-read": "Go to this page"

Bug: T183151
Change-Id: I0600dbc2e4d51a13675041d3c0741a793f4eae37
2018-01-16 18:36:54 -06:00
Stephen Niedzielski 8ca0cf2088 Fix: preview page URL for 404 RESTBase responses
Functional changes:

- Require page URL when constructing a PreviewModel null object. These
  models have valid titles and are used to display a preview when an
  extract is unobtainable. When presented with an empty URL, their
  linkage incorrectly pointed to the browser's current URL. Additional
  tests were added to verify the fix.

- Check missing title in addition to falsy response in RESTBase gateway
  and update the test assertion to check title. It isn't clear if this
  can happen in the wild.

- Forbid state mutation in the conclusion of
  MediaWikiGateway.getPageSummary() with a call to Deferred.promise().
  This is consistent with the rest of repo including RESTBaseGateway.

  http://api.jquery.com/deferred.promise/

Nonfunctional changes:

- Collapse two RESTBase gateway 404 tests into one as the scenarios and
  expectations were very similar.

- Add failure HTTP status to 'MediaWiki API gateway handles API failure'
  test stub HTTP response for consistency with other cases.

- Add nullity expectations to JSDocs touched and fix a couple typos
  throughout.

- Make the gateway tests a little more consistent by collapsing Deferred
  variable usage where appropriate.

This change is necessary to the completion of T183151 which uses the
PreviewModel null objects for additional error cases.

Bug: T183151
Change-Id: Ib77627fb9c80d8e806208bbafcfc615b130e3278
2018-01-16 18:36:48 -06:00
jdlrobson ff49e7627b Better error handling for unexpected responses
Bug: T182639
Change-Id: Iea04fe41b4be8e15927e93f16cbb4bb44328374f
2018-01-02 10:42:12 -08:00
Albert221 8feedf8368 Fix $.fn.hover is deprecated
Bug: T180861
Change-Id: I6a2cfbd26535c3e778a811e2f8ed5939123d6571
2017-12-06 00:52:19 +01:00
Baha 38a7978edb Schema: convert timestamp into integer
Mixed types such as integers and floats are causing issues with Hive.

Bug: T182000
Change-Id: I6643168a67207a59d56d7d39b2408587a5bb3524
2017-12-04 15:47:31 -05:00
Baha 58cdac8882 Schema:Popups - use revision 17430287
Event timestamp (as reported by `window.performance.now()`) is also sent
along with every action.

Bug: T180036
Change-Id: Ie3a648298005d51b8b4fbaa6a53e78caf50fa2d3
2017-11-17 12:49:22 -05:00
Volker E dc8f9c118b Use standard close icon
Making use of standard close icon and add `opacity` transition
for slight user feedback on states.

Bug: T50067
Change-Id: I258a50452eba8f9d0bfb550c2ad1a90ef7b6dc1f
2017-11-01 17:00:11 +00:00
Volker E 25c34072ec Align anonymous settings dialog appearance with style guide
Aligning anonymous settings dialog appearance with dialogs
elsewhere by
- setting base `font-size` to 14px to conform with Vector elsewhere –
  size is not inherited to the dialog as it's a direct child of `body`
- increasing contrast on descriptive paragraphs to conform with
  WCAG level AA
- reducing inner `padding`s slighty to conform with 8px grid
- aligning `border-color` and `box-shadow` values
- replacing static value with LESS variable

Also removing stylelintrc override rule which has been only around
for the old `box-shadow` value.

Bug: T178607
Change-Id: I738e0be11f3d1c94ea03288e0dddc1b983a6c729
2017-10-31 15:12:59 -07:00
Volker E 33a2eb9635 Optimize SVGs and align colors to WikimediaUI palette
Aligning SVGs to WikimediaUI color palette. Also optimizing
by help of SVGO and aligning them to each other, among changes:
- removing unnecessary, unused raster images
- removing invisible, application specific metadata
- unifying identation
- bringing attributes in order

Bug: T178257
Change-Id: Ief0ce99568e6a92700f2c8e4eb5990b989402389
2017-10-15 18:33:45 -07:00
jdlrobson 14e78466b2 Do not include @nomin instruction in dist build
When bundled by ResourceLoader this can interfere with debugging
of other concatenated files.

The nomin instruction introduced in 7bd29bb058
was meant to avoid RL minifying an already
uglified bundle, but that shouldn't matter here.

Bug: T177344
Change-Id: I90829668544e7c4ff7ddfdbb90d91b88a27a69f4
2017-10-11 14:34:18 -07:00
jenkins-bot c656264265 Merge "Align colors to WikimediaUI Style Guide and palette" 2017-10-09 16:55:59 +00:00
Ed Sanders 7f4dd1159b build: Update eslint and other linters
Change-Id: I0a20ad5d16d46afbc2a328f8e254295972f58ab0
2017-10-09 15:56:15 +01:00
Volker E df71836742 Align colors to WikimediaUI Style Guide and palette
Setting text color to `#222` and border color to `#a2a9b1` as
everywhere else.
Also making use of more recent LESS functionality with multiple
arguments per mixin to remove unecessary duplication of code and
change static values to central LESS variables where applicable.

Change-Id: I394c7e7e1369ff38b7ea91c7faebe773bcb2948d
2017-10-08 07:49:39 -07:00
smarita 3ec185cb01 Consider using more common image sizes for Page previews
Modified necessary files to increase size of popup from 300px to 320px

Bug: T173434
Change-Id: I47bbe9defe961008163551d5be4fc7b1ca08d0d1
2017-10-01 22:02:29 +05:30
Volker E 0c64c25c29 Unify box-sizing to common LESS mixin
Change-Id: Ifd3e7dd9be68c09b362c0fb8071cd9f27f846505
2017-09-21 15:32:09 -07:00
joakin 84e30f4613 Revert usage of Promise A+ in actions.js#fetch
.catch is only available with jQuery 3, which is only now starting to
roll out (see https://phabricator.wikimedia.org/T124742)

This patch rolls back the usage of .catch introduced in
https://gerrit.wikimedia.org/r/#/c/373327/ to use .fail, but only the
source part, given that patch introduced unit tests for this behavior

Bug: T174724
Change-Id: I6e1c342d812b35846061266bc59e493e87423fd8
2017-09-01 12:10:50 +02:00
Piotr Miazga f8498ce70c Store map files under .json extension
We cannot serve .map files from production servers. This makes
Popups extension difficult to debug. As a simplest solution we
decided to store map files as .json files so we can easily access
js maps files.

Changes:
 - changed webpack config to store map files under .json extension

Bug: T173491
Change-Id: Iaa55f75a8c5f3e8f1f169b3ac33241cc54f0413f
2017-08-29 21:18:35 +02:00
Piotr Miazga cb72bf4e82 Do not use keyword const as it's part of ES6 syntax
Changes:
  - instead of const use var

Bug: T174424
Change-Id: I3d786614b5acbfe9a05c332edd3a14c2e5afa417
2017-08-29 16:37:46 +02:00
Bartosz Dziewoński b63d2262f8 Don't use ES6 Number.isNaN
Number.isNaN is a new function introduced in ECMAScript 6.
MediaWiki only requires ECMAScript 5 supports from browsers.
Notably, Opera 12 does not have Number.isNaN. Instead, use
the global isNaN function (which behaves the same except for
non-numeric inputs).

Change-Id: If436cd26b21ce0336dfbc37144f6226e7b948e5e
2017-08-28 21:22:07 +02:00
joakin e865409593 Hygiene: Don't rely on .fail, use Promises/A+
Instead of using the non-standard old Promise implementation by relying
on .fail, migrate it to .catch.

In this specific case, where we were chaining promises with $.when, the
semantics from going from fail to catch actually change. .fail keeps the
promise in a rejected state, while .catch will change it to resolved
unless another error is thrown.

As such, when changing it to .catch, the catched error will be re-thrown
to keep the promise in a rejected state, so that $.when.then is not
executed.

Additional changes:
* Make actions.js#fetch return its promise for ease of testing.
* Test FETCH_FAILED, which was fully untested.

Bug: T173819
Change-Id: Ibd380b714586979c6e60b4c969d17f36a0796b52
2017-08-23 19:36:05 +02:00
jdlrobson e4e9bb3bd6 Popups A/B test infrastructure
Introduce PopupsAnonsExperimentalGroupSize config variable. This defines
a population size who will be subject to experimentation. If the group
size is undefined or 0 (default) and PopupsBetaFeature is false
(default value) Popups will be enabled for everyone. If it is any other
value, half that group will see page previews.

Drop the config variable PopupsSchemaSamplingRate - we will now only
EventLog when an experiment is occuring. This means we can simplify the
MWEventLogger class as shouldLog will always be truthy. Given server
side eventlogging is only used for preference changes
traffic should be low and not need sampling.

Introduce getUserBucket which determines whether a user is in a bucket
on, off or control based on the value of
PopupsAnonsExperimentalGroupSize. Add tests showing how these
buckets are calculated.

Caution:
A kill switch wgPopupsEventLogging is provided for safety.
It defaults to false. Before merging, please check if any config changes
are necessary.

Bug: T171853
Change-Id: If2a0c5fceae78262c44cb522af38a925cc5919d3
2017-08-17 21:07:07 +00:00
Piotr Miazga 910dd0408f if preview count stored in localStorage is not a number override it
There are some cases when the preview count stored in local storage
evaluates to NaN. When this happens we should override the value
to zero, store it in localStorage, and return it.

Bug: T168371
Change-Id: Ic44b7c7b5b716f6a0859f33278d56d2d95bbfb3e
2017-08-17 18:29:56 +02:00
Baha 3ac3769aa7 Remove event logging duplication detection and logging
We haven't seen the PP EventLogging instrumentation produce duplicate events
for weeks.

Bug: T172106
Change-Id: I6f3d7c0cdbf23161f63259e4d20d8a710376468b
2017-08-16 11:46:35 -04:00
Piotr Miazga d07441ec7f getPreviewCountBucket should return unknown when no bucket is found
Under some unknown circumstances getPreviewCountBucket() is called
with a value that is not a -1 or a natural number. When that happens
function returns 'undefined bucket' which causes eventLogging to
fail. I wasn't able to reproduce the issue, it might be specific
to browser/os. The safest way is to return 'unknown' for any other
case.

Bug: T168371
Change-Id: I374bb629762a86ac06a18e775d3c1a14682c9f55
2017-08-15 16:12:26 +02:00
joakin f37b76f8b4 Hygiene: make integrations/mwpopups pure
Instead of registering global variables in a function, make it pure
return the external interface and set it to mw.popups in the
src/index.js entry point.

Explicitly comment on index.js what is being set and why.

Bug: T171287
Change-Id: I94d467bfa7fa6e56033dd254518ad50b5dde5bfc
2017-08-09 16:07:08 +02:00
Piotr Miazga 8510b4b942 Allow 3rd party to check Popups enabled state by accessing mw.popups object
Changes:
 - introduced js module defining mw.popups object
 - introduced isEnabled() method which checks the redux store to retrieve
 isEnabled status

Bug: T171287
Change-Id: I523369831e2aa8a915ed1cb001b35d13b770f9da
2017-08-08 15:38:01 +02:00
joakin e6081106f1 Use EcmaScript modules instead of common.js modules
Why: Because they are the approved standard by TC39 and Ecma for
JavaScript modules.

Changes:
  * Wrap mw-node-qunit in run.js to register babel to transpile modules
    for node v6
  * Change all sources in src/ to use ES modules
    * Change constants.js to be able to run without
      jQuery.bracketedDevicePixelRatio given ES modules are hoisted to
      the top by spec so we can't patch globals before importing it
  * Change all tests in tests/node-qunit/ to use ES modules
  * Drop usage of mock-require given ES modules are easy to stub with
    sinon

Additional changes:
  * Rename tests/node-qunit/renderer.js to renderer.test.js to follow
    the convention of all the other files
  * Make npm run test:node run only .test.js test files so that it
    doesn't run the stubs.js or run.js file.

Bug: T171951
Change-Id: I17a0b76041d5e2fd18e2d54950d9d7c0db99a941
2017-07-31 23:05:44 +00:00
Volker E eebf17ac3f Remove obsolete vendor prefixed properties and at-rules
Opera 12.10 supported unprefixed animations, gradients, transforms &
transitions. See http://www.opera.com/docs/changelogs/unified/1210/
Removing support for Opera 12.0x versions.

Change-Id: I3476db173433c430f654e12ea1f17d2721410b83
Depends-on: Ie8edbcd7f85c713ea2156706ea3a3a7b423d8d9d
2017-07-31 21:39:42 +02:00
joakin 02507fb74d Hygiene: Move settingsDialog UI code to ui/
Bug: T171951
Change-Id: I58f77737456e1f4b9db6631f83e4b0f14212c939
2017-07-31 19:14:18 +00:00
joakin 31fa60d32c Hygiene: Move ui renderer.js to ui/ folder
Seems appropiate to group the UI portions of the source under a ui
folder.

Bug: T171951
Change-Id: I6d4317abea4e2a8e273e13fc40a7445bb54628ef
2017-07-31 19:11:41 +00:00
Sam Smith ba3c0b7f76 Hygiene: i13n: Return false over not sampling
Previously, if the browser didn't support the Beacon API, then
instrumentation/eventLogging#isEnabled would bucket the user with a
sampling rate of 0, which is equivalent to returning false. This change
simply does the latter.

Additional changes:
* Update the documented module names of the instrumentation/eventLogging
  and statsv modules.

Bug: T168847
Change-Id: I7ae5c10da42ca614b5b1a6619f9555e5665344cf
2017-07-26 17:06:58 +00:00
Sam Smith 8e00ecc5d1 i13n: popupEnabled = false for disabled event
... for consistency with the server-sent disabled event introduced in
I63faecb0.

Bug: T167365
Change-Id: I3a96df5279f6f0f4e573765735ab0e1fc6f406a8
2017-07-24 17:31:59 +00:00
jenkins-bot 59290bcf4a Merge "Remove duplicate events filtering" 2017-07-06 18:05:18 +00:00
jenkins-bot eebd0c5e8b Merge "Send disabled event from settings windows" 2017-07-06 18:05:17 +00:00
Piotr Miazga 426356e822 Remove duplicate events filtering
We had instrumentation for over 4 weeks and duplicate events rate
was very low. We want to keep stats so we check the duplicate events
rate but there is no need to filter those.

Bug: T167365
Change-Id: I72585beb21e9db589e45eeace657ef25f432abc9
2017-07-06 17:38:42 +02:00
Piotr Miazga 7e2c79ae0d Send disabled event from settings windows
Changes:
 - introduced new event 'disabled', sent from settings popup
 - added unit tests for 'disabled' event handling

Bug: T167365
Change-Id: I048b38122b8843199c86fd1ed9ec2ff21767e114
2017-07-04 19:08:37 +02:00
jenkins-bot 6a1176c0d2 Merge "Enforce no top&bottom margins on lists on page previews" 2017-07-03 13:52:28 +00:00
Piotr Miazga a82e54bf2d Override eventLogging to enabled when debug flag is on
In order to debug the EventLogging instrumentation in production
environments, we want to be able to bucket ourselves at will.
When the debug flag (?debug=true) is passed send all events
for given page view.

Bug: T168847
Change-Id: Id1b13b0ecaa791b4f26be4d1151bdbbe5270b64d
2017-06-30 18:52:51 +00:00
Piotr Miazga 06ef34c05f Enforce no top&bottom margins on lists on page previews
Changes:
 - set margin-top and margin-bottom to 0 on following elements:
   ul, ol, li, dl, dd and dd

Bug: T168941
Change-Id: I80478de046d7944fde3c0de3f96f5c9dc4623c36
2017-06-30 20:51:21 +02:00
Piotr Miazga 450e6bc34c Allow events without linkInteractionToken to be logged
Changes:
 - when event doesn't have linkInteractionToken do not check for
   duplicated tokens
 - hygiene, move event duplication logic into separate functions
   for better readability

Bug: T168449
Change-Id: I3ae197567ec9f67e104af109d4f1a1c1a6769d32
2017-06-30 18:03:40 +02:00
Baha def31b9e90 Remove paragraph margins from HTML extracts
This allows us to display an exact number of lines of texts in a preview.

Bug: T168332
Change-Id: I2066fbca2fce77c9eb1f0fe4278b96f13383e9a6
2017-06-20 16:11:22 -04:00
Sam Smith dcf8532cdf Hygiene: Group instrumentation modules
Following on from I4f653bba, since the schema and statsvInstrumentation
modules are similar, let's group/rename them:

  schema -> instrumentation/eventLogging
  statsvInstrumentation -> instrumentation/statsv

Change-Id: Ic59e0da7d4917f6733fd090f15d3c269af863f05
2017-06-20 11:41:37 +01:00
Sam Smith 67eb3b1dcf i13n: Log EL events with mw.track
Currently, the mw.eventLog.Schema class samples per pageview. However,
we expect that if a user is bucketed for a session, then all
EventLogging events logged during that session are in the sample.

Moreover, loading the class in the way that we did - asynchronously,
using mw.loader#using - introduced an issue where the eventLogging
change listener would subscribe in the next tick of the JavaScript VM's
event loop and miss the "pageLoaded" event being queued (see T167273).

Changes:
* Make the schema module follow the form of the statsvInstrumentation
  module, i.e. make it expose the #isEnabled method, and add the
  associated getEventLoggingTracker function.
* Update the eventLogging change listener accept the tracker returned by
  getEventLoggingTracker.
* Update/fix related JSDoc documentation.

Bug: T167236
Bug: T167273
Change-Id: I4f653bbaf1bbc2c2f70327e338080e17cd3443d4
2017-06-17 00:51:32 +00:00
joakin 98d9415361 Hygiene: Rename builder vars on require preview/model
preview/model is just a module/namespace object, not a builder class or
similar.

This patch changes a couple of imports to reference some of the exposed
functions of the preview/model where required.

In a shiny future, this pattern would be:

    const { createModel, createNullModel } =
      require( '../preview/model' )

Bug: T165018
Change-Id: If6ad4611538ca4f24e2443c0c3ed433275e995a6
2017-06-16 14:50:19 +02:00
joakin 010a4d91a6 Hygiene: Simplify gateways
gateway/*/rest were copies of gateway/restProvider just passing
a different provider. Docs were the same, they were untested, and
looking at them they seemed like unnecessary abstraction.

This patch removes the plain vs html structure, and separates gateways
like before, by endpoint.

There is a light utility in gateway/restFormatters.js that adapts the
call from the rest gateway to use formatters.js functions. It needs
testing, that I'll add in the next patch.

The flow for creating a gateway ends up as follows:

1. index.js calls gateway/index#createGateway( mw.config )
2. createGateway chooses based on wgPopupsGateway and invokes
  * mediawiki.js#createMediaWikiApiGateway or
  * rest.js#createRESTBaseGateway w/ restFormatters.js#parsePlainTextResponse or
  * rest.js#createRESTBaseGateway w/ restFormatters.js#parseHTMLResponse

Changes:
* Removed src/gateway/{plain,html}/rest.js
  * Extracted formatter functions to src/gateway/restFormatters.js
* src/gateway/plain/mediawiki.js -> src/gateway/mediawiki.js
         * tests/node-qunit/gateway/plain/mediawiki.test.js ->
           tests/node-qunit/gateway/mediawiki.test.js
* gateway/restProvider{,.test}.js -> gateway/rest{,.test}.js
* Change gateway/index.js#createGateway to properly call the rest
  gateways with the rest formatters

Bug: T165018
Change-Id: Ia75695dfc192aad5bc581a68882514bad6c29646
2017-06-16 14:49:59 +02:00
joakin b12599c871 Hygiene: Move createGateway to gateway/index.js
And add tests, given it is growing in complexity.

Additional changes:
* Interface ext.popups.Gateway -> Gateway in docs

Bug: T165018
Change-Id: I8a12333ad9d14d6a7fbde11afc42f607881e8ea3
2017-06-16 12:46:05 +02:00
joakin 490583bcb9 Hygiene: Capture jQuery at construction
and not from global scope all the time.

Bug: T165018
Change-Id: I90b55a65a7ca25c2998c811a98401feaeced165e
2017-06-15 20:10:08 +02:00
jenkins-bot a3b5bddefe Merge "i13n: Extract experiments module" 2017-06-14 20:09:02 +00:00
jenkins-bot 08fa97d8e0 Merge "Increase line number count of preview with tall image" 2017-06-14 18:44:17 +00:00
Sam Smith 6159af3151 i13n: Extract experiments module
... from the statsvInstrumentation module so that the bucketing logic
can be shared with other instrumentation modules.

Change-Id: I5732fa539a3911939fa85fa88c102fa8dcfa5613
2017-06-14 11:04:32 -07:00
Baha 686c1306ba Increase line number count of preview with tall image
Previews with tall image now have 9 lines of text as opposed to
the previous 7 lines.

Bug: T165978
Change-Id: Ib3d39f5164663f9c9ccaa3c0a974d524fc2f9463
2017-06-14 10:22:02 -04:00
Piotr Miazga f2fbef6ec7 Implement html/rest.js gateway which handles HTML Restbase responses
Refactor existing Restbase gateway and extract shared logic into
shared Restbase provider. Also introduced new createNullModel()
which defines an empty preview model.

Additionally improve naming in new gateways/formatter so function
names are more explicity.
 * Htmlize() became formatPlainTextExtract() as it should be used
   only with plain text extracts
 * removeEllipsis() became  removeTrailingEllipsis() as it removes
   only trailing ellipsis.
 * src/gateway/index.js defines gateways by configuration name stored
   in extension.json

Bug: T165018
Change-Id: Ibe54dddfc1080e94814d1562d41e85cb6b43bfc1
Depends-On: I4f42c61b155a37c5dd42bc40034583865abd5d7a
2017-06-13 20:19:05 +02:00
Timo Tijhof 4996886eb9 eventLogging: Use base 32 instead of 16 for fnv-encoded hash
Follows-up 79f3b318d0.

Number#toString supports up to Base 32.
Same collision behaviour but with a shorter string.
Typical length with Base 32: 7 (max: 11)
Typical length with Base 16: 9 (max: 14)

Change-Id: I91e91341cbecdec24549ace6a6300550f5b449ee
2017-06-12 21:43:05 +01:00
joakin 38b061a468 Return empty extract if string is blank after formatting
Bug: T167626
Change-Id: Icb89cbe447226850231c3d7d74e61e5f6a5db809
2017-06-12 12:59:31 +02:00
Sam Smith fb8d54c7ce actions/rest: Use DB-key version of title
This reduces the number of 301 Redirect responses when fetching previews
from RESTBase.

Bug: T167633
Change-Id: I830947ab79e72dcc023193412c8d5bcee986e23f
2017-06-12 11:22:55 +01:00
Piotr Miazga ef283c2509 Extract rendering/parsing mediawiki responses into separate class
Page Previews should be able to consume HTML response generated by
MediaWiki. First we need to move out plain text crunching from
renderer.js and model.js. Mediawiki and Restbase gateways will have
to parse/htmlize plaintext into nice HTML by themselves.

Bug: T165018
Change-Id: I5d7e9f610bb809aa9fb035a4a9f96e9e8796c9d8
2017-06-09 18:34:25 +02:00
joakin 002f4c8e0c Use delegated events in container
...instead of 1 event per link

Supporting changes:
* Delegate events on the container when booting up
  * Check eligibility of title on event triggered
  * Pass the title from the event handlers into the actions instead of
    storing it in the dom
* Add title.fromElement as sugar over isvalid(gettitle())
  * Not tested as it is sugar over the other 2 functions
* Fix action tests and integration tests

processLinks to be removed next

Bug: T165572
Change-Id: I4d9837706dc77ec64121ac94410c0d2da83692e4
2017-06-08 12:31:06 -07:00
joakin 356678ffcd Add title#isValid
Which checks if a title is an eligible one for showing previews

Bug: T165572
Change-Id: I5ade3fb84d400293d24de05e10119996e711b41e
2017-06-08 12:29:33 -07:00
Sam Smith 343506bdfb gateway: Fix Accept header sent by rest gateway
... and update the RESTBASE_PROFILE constant to the latest "stable"
profile for the endpoint.

Prior to this change the Accept header sent by the rest gateway was

  application/json; charset=utf-8profile="..."

This was discovered while responding to T166605.

Change-Id: I00f277e724c561634b26c9ab10bd35332c6dba91
2017-06-08 12:19:32 -07:00
joakin ef2f99ef65 Rename getTitle.js to title.js
In order to create a title#shouldShowPreview function next in the
module.

Bug: T165572
Change-Id: I9e59bb0f525d2698f882543ca0d4a1bde6b2d5d2
2017-06-08 12:10:14 +02:00
Sam Smith 98864a7ce3 eventLogging: Add missing properties to "tapped settings cog" event
When there's an interaction, then the "tapped settings cog" event should
have the same properties set as the other interaction-specific events.

This was discovered while QAing T164256.

Bug: T164256
Change-Id: I4749b52656203c7e0c42ae742556ee996eee322a
2017-06-07 10:13:23 +01:00
Baha 39672c5f49 Make vertical preview show 7 lines
Bug: T165978
Change-Id: I3474ab271679d290be58afca0d31c25b17b0442b
2017-06-06 15:43:21 +00:00
Baha f44218d2da Reduce height of horizontal gradient
Change padding to margin, and set the exact height of the container
to 7 lines.

Bug: T165974
Change-Id: Id4f97dddfb554ee2276b890a06030522076f8c88
2017-05-31 16:16:02 -04:00
jdlrobson 918a74b1e6 Docs: Don't register methods as globals in documentation
This ensures all files belong to a module.
When generating documentation no globals should be present

Bug: T158236
Change-Id: I134f38620fe46db11ba94dbede739f4336e0482c
2017-05-26 10:35:07 -07:00
jdlrobson e21a640002 Run doc generation on npm test
The benefit of this is if there are any problems with the
documentation they will not enter our codebase.

We do a similar thing in MobileFrontend

Bug: T158236
Change-Id: I30329dd868fe596c490f95354c3226c9cd4a2fc7
2017-05-26 10:26:35 -07:00
Sam Smith 30e616a224 doc: Document reducers/eventLogging module
Bug: T158236
Change-Id: I327f11ba804063a912c355477e9247a99e9b6e32
2017-05-26 09:02:47 +01:00
Sam Smith 66234e021e eventLogging: Add perceivedWait prop to all events
... and the previewType property as well.

Per the Popups schema [0], the "opened" action should have the
perceivedWait and previewType properties set.

Bug: T166323
Change-Id: I957d123434a6b750aff6f5279865321a08367382
2017-05-25 17:00:13 -04:00
Sam Smith b6bca56ad8 doc: Document userSettings module
Bug: T158236
Change-Id: Ia2365c7d6d1b9bc7438a3824656f1dde2a0170fb
2017-05-25 18:24:57 +01:00
Sam Smith 0670597434 doc: Document gateway modules
Additional changes:
* Fix the summary not showing up for the Container interface.

Bug: T158236
Change-Id: I7c5dee1b4525c2db28b89e57604d2d073620293d
2017-05-25 14:34:25 +01:00
Sam Smith e4f4041846 doc: Document statsvInstrumentation module
Bug: T158236
Change-Id: I138a1ef5305c0b49415d5f2a914ba45fd6aa869b
2017-05-24 10:45:49 +01:00
Sam Smith e9fb33af3a doc: Document isEnabled module
Bug: T158236
Change-Id: Ib80a5951e41765b982fecf4e2ac33767e4ef1738
2017-05-23 13:26:56 +01:00
Sam Smith 445318111b doc: Document counts module
Bug: T158236
Change-Id: I27106436e693289aa0c4140cef8e3d6210bcd8fb
2017-05-23 13:26:56 +01:00
Sam Smith 9350df7111 doc: Document schema module
Bug: T158236
Change-Id: I365fbfc1725d92003e902301252d53d4620c2128
2017-05-23 13:26:56 +01:00
Sam Smith af24fe958a doc: Document wait module
Bug: T158236
Change-Id: I5bdd3d7bb6fd0eb219b01ae113dc5bc8bdf12d0a
2017-05-23 13:26:56 +01:00
Sam Smith 5d9561bc91 doc: Document preview/model module
Changes:
* Assign exports to exports rather than reassigning module.exports so
  that JSDoc can guess inner members.
* Tidy up parameter types with JSDoc's "nullable" syntax.

Bug: T158236
Change-Id: I7261d1bb3924c9f14301490f54d7813dcffc959b
2017-05-23 13:26:56 +01:00
joakin 293d7ebe8d Clear interaction after an event for it is logged in EL
Given that interactions end up with an event logged, there shouldn't be
a reason to keep an interaction active after it's corresponding final
event has been logged. See Tbayer's state graph.

This patch removes the current interaction if an event with that token
is logged, effectively finalizing it and making it impossible for the
token to be reused from the state tree again.

Additional changes:
* Pass the logged event with the action EVENT_LOGGED so that the reducer
  can determine if it needs to do anything else.
* Since the interaction is removed, when undefined, guard against
  actions that use state.interaction freely. (Only allow BOOT,
  LINK_PREVIEW, and EVENT_LOGGED)

Bug: T161769
Bug: T163198
Change-Id: I99fd5716dc17da32929b6e8ae4aa164f9d84c387
2017-05-16 11:25:41 +02:00
Sam Smith cf0ea9db7b actions: Mix title and namespaceID into LINK_DWELL
This fixes a bug in I8a63d82, where the pageTitleHover and
namespaceIdHover properties of EventLogging events weren't being set.

Bug: T164256
Change-Id: Ie2c2d253f6508b89d48129fd17a902e5ded7cad5
2017-05-15 19:02:27 +01:00
Sam Smith 35bf613964 eventLogging: Add missing *Hover properties
Action changes:
* Include the namespace ID in LINK_DWELL.

Reducer changes:
* Add the createEvent helper, which adds the pageTitleHover and
  namespaceIdHover properties in the event.
* Create "dismissed", "dwelledButAbandoned", and "opened" events using
  the createEvent helper.

Additional changes:
* Store the target page's associated mw.Title using jQuery.
* Update the eventLogging reducer's test cases so that all LINK_DWELL
  representations include title and namespaceID attributes.
* Create the createStubTitle factory function, which returns a minimal
  usable mw.Title stub, and use it in the actions and integration test
  cases.

Bug: T164256
Change-Id: I8a63d82a65324680dff9176020a8ea97695428c4
2017-05-13 08:27:58 +01:00