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
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
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
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
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
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
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
Changes:
- introduced new event 'disabled', sent from settings popup
- added unit tests for 'disabled' event handling
Bug: T167365
Change-Id: I048b38122b8843199c86fd1ed9ec2ff21767e114
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
Changes:
- set margin-top and margin-bottom to 0 on following elements:
ul, ol, li, dl, dd and dd
Bug: T168941
Change-Id: I80478de046d7944fde3c0de3f96f5c9dc4623c36
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
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
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
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
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
And add tests, given it is growing in complexity.
Additional changes:
* Interface ext.popups.Gateway -> Gateway in docs
Bug: T165018
Change-Id: I8a12333ad9d14d6a7fbde11afc42f607881e8ea3
... from the statsvInstrumentation module so that the bucketing logic
can be shared with other instrumentation modules.
Change-Id: I5732fa539a3911939fa85fa88c102fa8dcfa5613
Previews with tall image now have 9 lines of text as opposed to
the previous 7 lines.
Bug: T165978
Change-Id: Ib3d39f5164663f9c9ccaa3c0a974d524fc2f9463
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
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
This reduces the number of 301 Redirect responses when fetching previews
from RESTBase.
Bug: T167633
Change-Id: I830947ab79e72dcc023193412c8d5bcee986e23f
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
...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
... 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
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
This ensures all files belong to a module.
When generating documentation no globals should be present
Bug: T158236
Change-Id: I134f38620fe46db11ba94dbede739f4336e0482c
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
... 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
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
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
This fixes a bug in I8a63d82, where the pageTitleHover and
namespaceIdHover properties of EventLogging events weren't being set.
Bug: T164256
Change-Id: Ie2c2d253f6508b89d48129fd17a902e5ded7cad5
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
It enables certain optimizations, see:
* https://webpack.js.org/guides/production-build/
Code is minified by uglifyjs.
Aditional changes:
* Add banner so that sources are not minified by ResourceLoader
Bug: T160061
Change-Id: I50c9148ebf2d860db42a24225bc128bfcfe56927
I6a38a261 made the eventLogging change listener count and discard
duplicate events and count duplicate tokens.
While we isolate the issue(s) that lead to duplication (reuse, likely)
of tokens, make the eventLogging change listener discard events with
duplicate tokens as well.
Bug: T161769
Bug: T163198
Change-Id: I0dbb16c37814d39d7aec35c8fb7cc7309704c550
Binding the behavior has been left out as it requires some refactoring.
Ideally, that functionality should be tested via integration tests, which
is already done.
Bug: T133022
Change-Id: If2fd472847eb3557de97c7ec9619e8831e9bda6d
Per the Popups schema [0], the "dismissed" action expects the
perceivedWait property to be set.
Happily, the time until the preview was shown is already recorded and
used to determine whether or not to create a "dismissed" or
"dwelledButAbandoned" event.
Reducer changes:
* When creating the "dismissed" event, set the perceivedWait property to
the already accumulated timeToPreviewShow.
[0] https://meta.wikimedia.org/wiki/Schema:Popups
Bug: T164256
Change-Id: I3fd253f1c2ff5fa8e24c9225060d728ffd8dfd27
Since Ieea378c9 all QUnit tests are run in Node.js and not in the
browser. Tidy up references to QUnit inside of the codebase and tooling.
Changes:
* Don't exclude src/processLinks.js in Istanbul code coverage reports.
* Don't test for window.QUnit in createSchema. We no longer run the risk
of sending beacons while running the QUnit test suite as it's no
longer run in the browser.
Bug: T160406
Change-Id: Ifb6adb84b8019dd69231b50af00bf978b708fc60
The tests were removed in Iae0a78d0b8a13353de70794b67387f2c3bab44c6.
The ultimate goal is to refactor the renderer code and make it
testable, but before doing so we need to add tests to cover the existing
code. This will give us confidence that we won’t accidentally break
anything when we refactor.
Some of the tests have been removed as the functionality covered by
those were moved to model.js in I20f29657fcf94101a71ed13c0920508db71292ce.
Bug: T133022
Change-Id: I7b20324dd5fe8a428cdd96959b65bc82d44fb515
The ultimate goal is to refactor the renderer code and make it
testable, but before doing so we need to add tests to cover the existing
code. This will give us confidence that we won’t accidentally break
anything when we refactor.
Bug: T133022
Change-Id: I3ecbfb9bb3ac9c63fdd40df502796748c62949fe
The tests were removed in Iae0a78d0b8a13353de70794b67387f2c3bab44c6.
The ultimate goal is to refactor the renderer code and make it
testable, but before doing so we need to add tests to cover the existing
code. This will give us confidence that we won’t accidentally break
anything when we refactor.
Bug: T133022
Change-Id: I897276a1a953f6be62e4c2d4a24e0f22fc6ef141
With the final goal to remove the real mw stubs, move the processLinks
tests away from test cases, and split getTitle to be tested standalone.
Supporting changes:
* Move getTitle out to it's own module
* Will be tested separately in a followup commit
* Remove global declaration of mw.popups.processLinks (not needed any more)
Bug: T160406
Change-Id: Ieebd1257a2476081c67a318d3f05dffa1d3b9bdd
The eventLogging change listener is responsible for ensuring that the
internal state of Page Previews matches its external state (that
perceived by the user and UA). It does this by logging events with
ext.eventLog.Schema#log. This makes it the perfect place to track and
discard duplicate events enqueued by the Page Previews codebase observed
in T161769.
Make the change listener track events that it's logged by storing hashes
of the dynamic parts of them in memory. If the eventLogging change
listener sees the same event more than once, then it discards it and
increments a counter in StatsD.
This behaviour should be enabled for a matter of days as we should see
whether the duplicate events are being enqueued by the Page Previews
codebase immediately.
Bug: T163198
Change-Id: I6a38a2619d777a76dd45eb7300079e1f07b07b12
The statsv change listener depended on both the analytics tracking
function and whether it should log metrics to StatsD. We can simplify
the behaviour of the change listener by passing in a function which
doesn't log metrics to StatsD if such logging is disabled.
The change listener is now more isolated from other components.
Moreover, sharing the analytics tracking function with other components
is simpler as there's no repeated code.
Bug: T163198
Change-Id: Ibf4785fa4c27c1ad4739f02410f57412f56ff481
... by running the following:
svgo --disable removeXMLProcInst --pretty --folder images/
svgo --disable removeXMLProcInst --pretty --folder resources/ext.popups.images/
Also, add an XML declaration to resources/ext.popups.images/close.svg so
that it's recognised as an SVG by libmagic. This isn't strictly an issue
because of the way the SVG is served by the ResourceLoaderImageModule RL
module but it's consistent with the other SVGs.
Change-Id: I10b2286d6577701ba3b9a8651d5165fa81b8d293
If the user dwells on a link for long enough, then the gateway makes a
request, which is allowed to complete regardless of whether or not the
response is required.
If the user abandons the link but the request completes before the
abandon completes - currently, ABANDON_END_DELAY is 300 ms - then the
preview will be rendered temporarily.
Fix this by not rendering the preview if the user has started to abandon
the link.
Bug: T163350
Change-Id: I154dde4e3ccaed3d11cb023c85c44451fc0ad957
* Merge mwe-popups-icon with mwe-popups-settings-icon
* Remove PNG - now generated by the ResourceLoader module
* Adjust popup footer paddings/widths and store them in variables
* RTL and LTR compatible
Bug: T133956
Change-Id: I14ccd7b6731e9ec49f9959411fd17f7c9fdf43be
When the user dwells on a link and there's enough room to display a
preview above it, then the preview should rest atop the link rather than
above the mouse.
Bug: T161366
Change-Id: Ia7266f6e5c272817581bdbcb3710429b266556e4
If the user CmdOrCtrl+Clicks on a link, then the link will remain
focussed. If a preview is shown, clicks another element on the page, and
then there'll be no token to include in ABANDON_* with.
Bug: T162924
Change-Id: Ie2237aa55ea9a11070498b66c73b8bf1898d8d44
I09d8776 introduced a bug in the eventLogging reducer where reducing
LINK_CLICK would remove any accumulated interaction state but not close
the interaction.
Update the associated tests so that they correctly test the reduction of
this action.
Bug: T162924
Change-Id: Ia03e719c228ee96f279c1fa89252dc6b6371a8e9
... the interaction.
Following on from Iccba3c4c, LINK_CLICK shouldn't be considered the end
of an interaction because the link or preview can still be abandoned by
the user. Unlike ABANDON_END and LINK_DWELL, therefore, LINK_CLICK
musn't destroy the interaction but indicate that no more events should
be enqueued for the interaction.
More concretely, if the user has clicked the link or the preview, then
a "dwelledButAbandoned" or "dismissed" event shouldn't be logged.
Changes:
* Distinguish between "finalizing" and "closing" an interaction, where
the latter is the current behavior of ABANDON_END, LINK_DWELL, and
LINK_CLICK, in the eventLogging reducer and associated tests.
* If the interaction is finalized, then either the "dwelledButAbandoned"
or "dismissed" events shouldn't be logged.
Bug: T162924
Change-Id: I09d8776da992053f89a77508e29a7cde3cfeeac6
As noted in T160406, the only QUnit tests that requires a running
MediaWiki instance is the test for mw.popups.processLinks. The function
itself is isolated from the rest of the codebase.
Now, as noted in T162876#3182198, during boot the
ext.eventLogging.Schema module is loaded asynchronously with
mw.loader.using. Since boot is unconditional and happens ASAP this
happens when the tests are loaded and run.
In the short term this can be avoided by not making the tests depend on
the entire codebase. The long term solution is laid out in T160406.
Supporting changes:
* Bundle assets with webpack@2.4.1.
Bug: T162876
Change-Id: If1ee1853ba7a9b2a66b24bb93b4e6062b92b0dba
... in the eventLogging reducer.
Like ABANDON_END, the LINK_CLICK should be considered the end of the
interaction in the context of the EventLogging instrumentation, i.e. no
events should be logged after the user clicks the preview or the link.
See T159490#3172692 for additional context.
Bug: T159490
Change-Id: Iccba3c4c2b6121016ff7923c11b1622bc046ad6b
Like the FETCH_COMPLETE and ABANDON_END actions, the PREVIEW_SHOW action
was delayed but not conditionally reduced. As ABANDON_END is delayed,
there's a potential for a race and if ABANDON_END is reduced before
PREVIEW_SHOW, then there's no interaction to reduce the action into,
which causes an error, e.g. T159490#3165276 and T162373. Making
PREVIEW_SHOW require a token stops the error occurring in this scenario.
An alternative would be to clear the timeout created in
ext.popups.Preview#show in #hide. However, this would be inconsistent
with actions#fetch and actions#abandon.
Bug: T159490
Change-Id: Ibd2c0c6f45e4392582cc6ed08517f6ca1146d57a
Mirroring all other actions that are dispatched after some delay, add
the token to the PREVIEW_SHOW action.
Supporting changes:
* Pass the token to ext.popups.Preview#show so that it can be passed to
actions#previewShow.
Bug: T159490
Change-Id: I128fd56e770ed09d5d0dc55db73d11b013049c79
... instead of using the active element.
In the case of the eventLogging reducer, this fixes scenario 4.1 from
T159490#3150331, which was caused by late FETCH_COMPLETE actions being
reduced regardless of whether interaction had been finalised or a new
interaction had started.
Bug: T159490
Change-Id: If9d718625b0302ea2f75a778005643b4eef62bde
Treat these responses not as an API failure. Show a generic
preview whenever the server responds with a 404.
Bug: T160744
Change-Id: Id6169d9d4c7493f5b6511cc78fe65d448cdadc03
I6d9ff52b introduced a regression where if a logged out user clicks the
settings cog then an error is thrown.
For now, passing the event to the behavior for further processing is
required. However, it's clear that this makes the
ext.Popups.PreviewBehavior abstraction leaky.
Bug: T162324
Change-Id: I9dea04eb7435f9349e60d477f5701ec5dd655ebd
