Commit graph

991 commits

Author SHA1 Message Date
Thiemo Kreuz bb7ef8f205 Remove unused attributes from .svg files to optimize them further
An id="…" can potentially be used to target elements individually, manipulate
them, or show them in another context. This is done with the elements in the
file pointer-mask.svg. But not with any other image.

The file-rule="…" only has an effect on shapes that intersect with itself.
This is not the case for this basic "L" shape.

If you think this is fine, I might look at the original OOUI icons as well
and apply the same optimizations on them. But there are 800+ of them to be
looked at, and I want to invest the time only if you think it's worth it.

Change-Id: Ifa4bd60a436e58d3a7c1536f8201fe76fbdac57c
2019-02-20 11:46:13 +01:00
Thiemo Kreuz fe7c107f8b Reference gateway accepts .mw-reference-text and .reference-text
While reviewing the code of the reference endpoint I found it encodes
two CSS class names, "mw-reference-text" and "reference-text". So far
our scraping gateway only implemented one.

https://phabricator.wikimedia.org/diffusion/GMOA/repository/master/

This patch also extracts a piece of code to a named function. This makes
it much easier to read, I feel.

Bug: T214908
Change-Id: I9d1bb1f4c21eb9d57a6b763ca1f756e6cf7049e0
2019-02-20 11:38:51 +01:00
Thiemo Kreuz d81415a040 Add all reference type icons and messages
These icons are currently unused because the gateway does not deliver
the necessary information. This will be used starting with I6223cbb.
This patch aims to introduce all resources needed by the later.

Bug: T214908
Change-Id: Ie0c3c059222700169f2605c3123554c74d974256
2019-02-20 09:32:11 +01:00
Thiemo Kreuz 9438210889 Teach the title parser to always accept self-links
The definition of "self-link" in this context is an <a href="…">
element that points to the exact same URL as the current document's
location, excluding a possibly different #… fragment. This is typically
the case when the <a> element does not contain a full URL, but something
like href="#fragment". JavaScript's HTMLAnchorElement.href getter
automatically expands this to be a full URL.

Example:
var a = document.createElement( 'A' );
a.href = '#test';
console.log( a.href );

Notes:
* This new code assumes the wgPageName setting properly reflects the
  page name requested via the current document's location. Core does
  give us this guarantee.
* The only URL element that is intentionally not compared is the
  protocol.
* This accidentially fixes T215899 as well, because the namespace check
  is now bypassed for self-links (as it should).

Bug: T214861
Bug: T215899
Change-Id: I2670331cbbdebf7dc9fc70d7342724534f9f54ec
2019-02-17 15:05:14 +01:00
Thiemo Kreuz fb5b120515 Minor fix-ups to type hints in JavaScript code
This patch also removes misplaced empty lines at the beginning of a
scope. In PHP code we even have a sniff for these. In JavaScript we
don't, but I suggest to be consistent about this.

Change-Id: Ic104ae8fe176da1dafa9bc783402adecb71de1f0
2019-02-14 11:15:01 +01:00
Thiemo Kreuz 9ee61e18b5 Fix <clipPath> capitalization in pointer-mask.svg
According to the standard.

Note: In a previous patch I removed the <?xml …?> line. This conflicts
with what https://www.mediawiki.org/wiki/Manual:Coding_conventions/SVG
suggests. However, I think the removal is ok in this particular case
because this file will never be shown as an image, and never be shown
standalone. Instead, it will be inserted in the documents DOM. The XML
header doesn't matter anyway then.

Change-Id: If23ad54985abb30f8c92500546bd04eeca44fab3
2019-02-13 23:27:59 +01:00
Johannes Kroll d4d8cefac4 Rename link in footer to "Jump to footnote"
Bug: T215063
Change-Id: I18201bdeee65418ef147203a845f34447c24e163
2019-02-13 16:01:24 +01:00
Thiemo Kreuz a268d1e727 Minimize/optimize pointer-mask.svg file
I decided to keep the comments because they are sooo helpful, but
tried to shorten them a bit. The biggest change is the indention with
tabs and the much more compact <path> elements. The shapes are the
exact same. I manually confirmed this for all four.

Change-Id: I2d1294c9ae7e398dcbe2d111c42848d17be8a67e
2019-02-13 13:55:25 +01:00
Thiemo Kreuz 90acf2a778 Fix pointer mask and CSS offsets for flipped popups
I tested this with all 16 possible combinations:
* The pointer can show up in all 4 corners.
* The popup can contain a thumbnail or not. The code for the pointer
  is very different then, because the SVG masks are only relevant in
  this scenario.
* The thumbnail can be tall or not.
* I even tested tall popups without a thumbnail. This is a combination
  that is impossible in production scenarios.

I found 3 issues. This patch fixes 2 of them:
* The pointer is misplaced in the bottom-right corner when the popup
  does not have a thumbnail (as reported in T215194).
* The pointer is misplaced in the upper-right corner when the popup
  shows a thumbnail.
* The pointer in the upper-right corner is gray instead of white when
  the popup is tall, but does not show a thumbnail. As this is not
  relevant in production, I did not fixed it.

It seems both misplacements are because of the same reason: For some
reason, calculations are done based on the assumption the popup would
be 300px wide, but it is 320px wide.

Note I did not just added 20 everywhere, but manually alligned the
pointer triangles so they are placed exactly the same distance from
the corner as in the three other corners.

Note I did not tested this (yet) in RTL scenarios.

Bug: T215194
Change-Id: If0ca63d4d4b6e8083c7de1517fe32f49671a40e6
2019-02-13 13:48:51 +01:00
Thiemo Kreuz 33cb635e53 Improve documentation of the previewType constants
As well as:
* Simplify the selectGatewayType() function a bit. Or was this
  intentional?
* Remove a TODO we don't need any more, at least not in this file.

Change-Id: I5528f0012cbaf8b4e88e22c7e2a8d87bf027e8f1
2019-02-12 17:17:31 +01:00
Thiemo Kreuz 5bc4ed8f9f Exclude links in popups to trigger other popups
We discovered a bunch of possible solutions (see previous patch sets),
including replacing the `$( document )` selector with a more specific
one. That idea does not pass the linter.

Very late I realized the original selector starts with
`#mw-content-text`. This heavily limits where popups are allowed to
appear: really only in the main text content area.

We should limit reference popups to the exact same scope.

This fixes the issue described in T215195. Before, the content of the
popup was covered by the selector. Reference links *inside* the popup
would trigger another popup, which makes the current popup disappear.
Now the popup itself is not covered by these event handlers any more.

Bug: T215195
Change-Id: I142aee68abbd57ca321873855fef9209e0db0bbf
2019-02-12 13:47:27 +01:00
Thiemo Kreuz 0686f40069 Move possibly expensive title parser construction down
There are a few cheap checks done before this regular expression is
even needed, most notably the check for a pretty URL (without query
parameters). Since the vast majority of links processed by this parser
are pretty, I believe this optimization is worth it.

Change-Id: I730b87dc010161e8bc3f311c517293c0ad553326
2019-02-11 19:07:02 +01:00
Thiemo Kreuz d45c6cd273 Move thumbnail rendering code into pagePreview renderer
This is now possible since the render functions return jQuery objects.
All this code is exclusively used in the pagePreview.js file, and
doesn't need to make the already very big renderer.js file even
bigger.

Note the tests for all renderers have always been collected in a
single file. That's why the test case does not move.

Change-Id: I0c24638751c5f0e93d2bc0f3f4bb61fa0cf50d15
2019-02-08 11:30:00 -07:00
Thiemo Kreuz 7383a4e1ef Make renderSettingsDialog() return a jQuery object
For consistency with the other render…() methods that now all behave
the same.

Change-Id: I0fe581e6d1daeafc969ec4bfbcf18b363f702475
2019-02-08 16:08:05 +01:00
WMDE-Fisch 12a8aa3a86 Trigger click on source footnote link
To make sure that we enable the link highlighting in the Cite extension we want
to trigger the click handler on the original footnote link. This is done by
passing the id of the source element to the model and the renderer.

Bug: T213905
Change-Id: I0bd59ac326269f3c0850946851fb79b611dc2a57
2019-02-07 14:15:45 -07:00
WMDE-Fisch 0b64e803f2 Move gateway typedefs to gateway index
When we started reorganising the code around the gateway selection
we forgot moving the docs and typedefs from the (now) more specific
page.js file to the (now new) index.js. - This patch fixes that.

See also I5efa9fb8f63f1487c627eb9a3f1fe47f43c611cc

Change-Id: I7b3582882247e22497d3a37b896a8a7c9950cf0d
2019-02-07 16:29:04 +01:00
Thiemo Kreuz fb56a2e729 Mark reference preview content area as "mw-parser-output"
This CSS class is used by the regular wikitext content area, and used
by certain CSS selectors (e.g. the external web link arrow).

The DOM structure for each popup is (intentionally) created outside
of this scope, at the end of the current page's body. This works
great for page previews because the do *not* want to share any styles
with the rest of the page. But reference previews want to do this.

In this patch I also remove the inner <span>. It was misplaced (note
the name) and resulted in block elements nested in an inline element.

Bug: T214463
Change-Id: I740e37a2ed929edf971b348fbf20e5fb12012d37
2019-02-04 12:37:11 -07:00
Thiemo Kreuz c35715bdec Make all render functions return jQuery objects instead of strings
This gets rid of a little bit of code duplication, and makes the
interfaces all conform to one standard again after I05ed4b8 left them
in a little inconsistent (but properly documented) state.

Bug: T214970
Change-Id: If8407c1a48aff1cb31fc2e74b3c2b846e79a3cb5
2019-02-01 12:49:53 +01:00
Thiemo Kreuz 67ceeaeeed Inline code setting target="_blank" in renderReferencePreview()
As discussed in Iaadcce9. This does have a few benefits:
* Less code in the already pretty big render.js file.
* The code setting the target attribute is much closer to where it
  belongs: in the file that specifies how the content of a reference
  popup should look and behave.
* The class name "mwe-popups-extract" is not mentioned in two different
  files, but in the same.

Note this changes the signature of this src/ui/templates/… file to not
return an HTML string any more, but a jQuery object. The other templates
still return strings. I believe this is fine, and not that much of a
difference anyway. The signatures don't need to be identical. And the
jQuery object still represents the exact same HTML as before.

If it helps we could change all templates/… signatures accordingly.
Could be done in this or a separate patch.

Bug: T213908
Bug: T214970
Change-Id: I05ed4b886f79c5ae748f53ab9fed965dfd217620
2019-02-01 12:46:29 +01:00
Thiemo Kreuz 03ef969122 Show reference previews only on self-links
This solves the (I believe) only regression we introduced: A bad fake
reference like <span class="reference">[[Other article#Section]]</span>
showed a page preview for the "Other article" before we introduced
reference previews, but would have shown nothing after I9ec57e0.

Checking if the link is a self-link solves this and possibly more related
issues. Only self-links can point to a footnote on the same page.

Manually created fake-references like
<span class="reference">[[#Section]]</span> still have a chance to show
nothing in case the manually created HTML does not strictly follow the
expectations in the gateway. There is not much we can do about this. We
should not accept any arbitrary HTML but need to make at least *some*
assumptions.

Bug: T214970
Change-Id: I86e91bf45c3ae4c6a4086f7f1c7b1280fd400d17
2019-02-01 12:14:06 +01:00
Thiemo Kreuz f1fd3eafc4 Mark optional elements in the PreviewModel specification as such
We updated this documentation just recently via Ie370cfe. We followed
what the createModel() function does. But this is not the only way a
PreviewModel object can be created. Reference previews, for example,
don't use it.

Instead of following createModel(), I checked what the different popup
types actually use.

Bug: T214970
Change-Id: I2c4293a48387836dc30e18d10d952b4a26e6f2b5
2019-02-01 11:50:43 +01:00
WMDE-Fisch 0c0226c4ee Add and fix gateway/page module
Change-Id: Icd8e9e3a6f643ebba0c2bb9b4fcb84e1260d41ca
2019-02-01 11:14:00 +01:00
WMDE-Fisch dbb7e0fbdc Factor out gateway selection
This is mainly done to increase testabilty of this part. I am a bit
unsure if this should ( have been ) integrated in the former index.js
that's now the page.js. - See also the refactoring done before.

Bug: T214971
Change-Id: I90d0441510bc1ec0b4900a392afcbaff6a552377
2019-02-01 10:14:00 +01:00
WMDE-Fisch 31ee16938c Rename page gateway file
This is as preparation to introduce a gateway switch that decides if the
page or reference gatway should be selected. Moving that code to it's own
realm makes that path better testable.

Bug: T214971
Change-Id: I5efa9fb8f63f1487c627eb9a3f1fe47f43c611cc
2019-01-31 12:09:12 +01:00
Ed Sanders e1c4e94b23 build: Update eslint-config-wikimedia to 0.10.0
Also enable jquery ruleset.

Change-Id: Ie1f43d0335ea2aad1e2dd5d86b775316105c3d90
2019-01-31 11:05:33 +01:00
Thiemo Kreuz 515775685c Fix a series of issues with misdetected reference elements
This installs a series of safety nets:

* The selector [href*="#"] skips links without a fragment.

* It's still possible that a fragment exists, but is empty.
mwTitle.getFragment() checks this.

* The gateway does not assume the element exists, but checks this first.
If there is no such element, the gateway aborts the request in a way
that no error popup is shown. This is currently only possible with the
`{ textStatus: 'abort', xhr: { readyState: 0 } }` response as seen in
this patch. We might need to introduce a new, more clean way to silently
quit a fetchPreviewForTitle() call.

* The test for the reference gateway finally covers the scraping code.

Bug: T214970
Bug: T214971
Change-Id: I9ec57e0fbb0d21beaaa7b359c1c2bef64d2c14f5
2019-01-31 10:29:46 +01:00
Thiemo Kreuz a8859658f5 Add missing HTML escaping to all existing page preview types
Including tests for all situations.

I believe it is impossible or extremely hard to actually abuse any of
these places. All these data are not extracted from the current page, but
delivered either by MediaWiki's api.php or a RESTful endpoint, as
configured via $wgPopupsGateway and $wgPopupsRestGatewayEndpoint. A
possible attacker would need to write it's own endpoint (which must either
run on the same server or somehow ignore the CSRF token), and set the
value of mw.config.values.wgPopupsRestGatewayEndpoint on the client to
this endpoint – which requires just *another* attack vector to be able to
do this.

It's "the right thing"(tm) to escape all this anyway.

I found two possibly relevant security reviews of this extension, T88171
and T129177, resolved in 2015 and 2016.

Bug: T88171
Bug: T129177
Bug: T214754
Bug: T214971
Change-Id: I1d118c9ccaea434a253a772d18139b9b077118ab
2019-01-30 18:29:14 +01:00
Thiemo Kreuz 0a8f591212 Rewrite title module to preserve all link's #fragments
This will affect all links, including [[Other page#Fragment]] for
example. But it will not have much of an effect there. The mw.Title
class is able to understand strings like "Other page#Fragment". All
old code calls title.getPrefixedDb() on the result. This will *not*
include the fragment. Only the new code will use title.getFragment().

I made sure this does not affect regular page previews, even when the
link is something like [[Other page#Fragment]].

Bug: T213415
Change-Id: I15611a44aa0477cc5e48ee4b12aae3cd981d977c
2019-01-29 17:43:28 +01:00
Thiemo Kreuz 0c889c4cd4 Add default OOUI reference icon to all reference popups
I tried hard to keep the CSS as small and robust as possible. The
icon will be align with the text by adding a negativ margin. With
that we also decided against using RTL and LTR specific icons that
are positioned at the edge of the canvas for now.

Bug: T213907
Change-Id: I98888114e1c50e249cf31e71749323bd4f69da3f
2019-01-29 17:06:40 +01:00
Thiemo Kreuz bb60d5b716 Move default "Footnote" title from gateway to renderer
I guess both is fine: either having the default in the gateway (as it
was before), or in the renderer (as this patch proposes). I, personally,
feel better with having it closer to where it is needed. This way it's
not possible to accidentially deliver a model object with an empty title.
The renderer will catch this.

At the moment we don't know exactly how we will fetch other titles (e.g.
"Book").

This change is split from I15611a4 where it was a little misplaced.

It also includes a test for the default fallback title.

Bug: T213907
Change-Id: I8ec3ddc21a417da7f95feff7b080cbd60d5472e7
2019-01-29 11:37:47 +01:00
Thiemo Kreuz 6e5be9d2ef Add missing HTML escaping to reference preview renderer
Including tests. I also changed the title to include quotes as well,
even if not critical in that case.

Bug: T214754
Change-Id: I2f92a5714f7adc229a003f9167bcc9afdbc55583
2019-01-28 19:35:20 +01:00
Thiemo Kreuz 46cda9fa44 Add missing escaping for jQuery ID selector
Special characters that have a meaning in one of the many different input
formats jQuery accepts must be escaped.

The real-world use-case are references like <ref name=":1"> with a colon.
But it's many more characters that need escaping. See
http://api.jquery.com/category/selectors/

Note this patch misses a test. I already uploaded I9ec57e0 to fix the
currently incomplete tests. But I can't make it work. How do I create an
element in the test environment so that jQuery finds it?

I suggest to merge this and continue working on the tests later, because
this is currently one of the most annoying issues that makes all testing
unreliable.

Bug: T214710
Change-Id: Ifb5fe896936078f799298ac803d019d9caa048c8
2019-01-28 15:18:01 +01:00
Thiemo Kreuz 0859f2ed8d Also set rel="noopener" on target"_blank" links
The need for this is more a sign for a broken specification than an
actual issue with this code. But better be sure than sorry. More
details at
https://mathiasbynens.github.io/rel-noopener/

Bug: T214776
Change-Id: Idbcfae6d146fbbe3bff730239329beeb3455e18c
2019-01-28 12:04:40 +01:00
Thiemo Kreuz 8d8446571e Open all links in a reference preview's content in new tabs
Bug: T213908
Change-Id: Iaadcce99b68542094333730d99f776d9e5f056f9
2019-01-25 14:00:17 +01:00
Thiemo Kreuz 97a5d335d7 Replace rare {!…} and {?…} JSDoc syntax
This is documented at http://usejsdoc.org/tags-type.html, but not in many
other places, especially not in the JSDuck documentation.

The {!…} syntax means "can not be null". This is the default anyway.

The {?…} syntax means nullable. In a few situation is was used when a
parameter can be undefined. I decided to remove it everywhere and replace
it with {…|null} when appropriate, because this is much more explicit. Less
syntax to remember.

Note I'm intentionally not using the […] syntax when a parameter is followed
by non-optional parameters. Actually skipping a parameter in such a situation
would mess the parameter order up. Having optional parameters not at the end
is sometimes used as a feature in JavaScript code, but not in this codebase,
as far as I can see.

Change-Id: Ie370cfe08c32d1af5b0341951bed044fc3511c57
2019-01-24 21:00:45 +01:00
Thiemo Kreuz 553e76e2bc Add QUnit tests for most new reference preview code
Excluding tests for the renderer which keeps failing. This will be
readded in a later patch.

Bug: T213415
Bug: T213908
Change-Id: If79fa3d0a7a20f121b1ceda6e0e33ad691b1ad30
2019-01-24 19:35:38 +01:00
Stephen Niedzielski 214795f829 Hygiene: move imports above constants
Make the imports more consistent by moving them to always appear above
declarations.

Change-Id: Ic1b6786d5daccf71898aeab3cb0ec2b64efe80f7
2019-01-24 08:33:29 -07:00
Thiemo Kreuz 57fd85fc68 Rename getPageSummary to fetchPreviewForTitle
It's not exclusively about page summaries any more.

We had a few suggestions in mind:
* get, fetch, request, or issueRequest. But I feel these are all to
  generic and don't describe well what the method does. As a reminder:
  It expects a Title object and returns a promise, which returns a
  PreviewModel object, which contains an HTML "extract".
* fetchPreview? I feel this can still mean to many things.
* fetchPreviewModel? But we don't really need to repeat that it will
  return a model object.

So I went for fetchPreviewForTitle. What do you think?

Bug: T213415
Change-Id: Icb32c63cec82f72453dc1507c9f8b8d461fd4f4c
2019-01-23 17:50:19 +01:00
Thiemo Kreuz 671c39ef4a Add reference preview type
This adds support for preview popups on reference/footnotes from
the Cite extension. For that a new preview type was introduced and
integrated into the existing structures.

The essential starting points were this code comes into action are
added behind the feature flag introduced in the previous patches.

Bug: T213415
Change-Id: Ie0ccb03117bd654373d0f458b62cc52018361c67
2019-01-23 12:12:36 +01:00
Thiemo Kreuz 197aefda3c Do not export private renderer functions
Most of the code is exported because is is tested separately. But
these are all tested via createPreviewWithType(). I think it's just a
minor mistake to have these exported.

Change-Id: Ic4f4dc40fd95a60aba45cb5aa3fcbb6e3bc8c386
2019-01-21 13:26:11 +01:00
Thiemo Kreuz 170ab5422a Rename current gateway to pagePreviewGateway
This is split from the current draft patch Ie0ccb03. This is part of a
series of very small patches that prepare the code for new types of popups.

We decided to not add code for other types of popups to the existing
createGateway() function, but introduce new files and functions instead.
Renaming, for example, the existing `gateway` variable name will make it
much more obvious which of the future gateways does what.

Bug: T213415
Change-Id: Ifcbc3ba53d0ab9ef67adf1f314defc76b4f89e89
2019-01-17 17:15:22 +00:00
Thiemo Kreuz 3b846cd472 Introduce TYPE_REFERENCE constant in advance
This is split from the current draft patch Ie0ccb03. This is part of a
series of very small patches that prepare the code for new types of popups.

Bug: T213415
Change-Id: I4f4392057f6d3eff78409c8b6f49898c8be45d3e
2019-01-17 17:13:55 +00:00
Thiemo Kreuz aaa40b4fbe Also exclude the Cite extension's "Jump up" backlinks
This became an issue after the patch Ifa56d41 (part of T206323) removed
some hidden <span> and replaced them with title="…" tags. The CSS selector
in src/index.js get's active on all <a> elements that have a title="…"
attribute. This is now the case for all references that are used one time
and have their ↑ (or ^ on the English Wikipedia) linked.

I realized this is really only an issue for these ↑ links.

The more general issue described in T198652 still holds true, but becomes
less urgent with this bugfix.

Bug: T198652
Bug: T206323
Bug: T212419
Change-Id: I9287e8692d031f9d2ba50f967520bf327ed5c42f
2019-01-17 17:16:35 +01:00
Thiemo Kreuz 2f2286921d Change getPageSummary() to use mw.Title object instead of string
This is split from the current draft patch Ie0ccb03. This is part of a
series of very small patches that prepare the code for new types of popups.

Bug: T213415
Change-Id: I00d46a716c0e6ada82ffc0034a7dd5582363c657
2019-01-17 15:17:28 +01:00
Jan Drewniak 83a28d177f Storybook.js for Popups
Storybook.js provides a framework for
viewing and working with UI components.
https://storybook.js.org/

This patch adds the Storybook.js UI library to Popups for
the purposes of viewing multiple previews at once.
This enables viewing page previews in the following states:

- with thumbnails
- without thumbnails
- with SVG thumbnails
- with narrow thumbnails
- with white background thumbnails
- in RTL languages
- in non-latin languages
- disambiguation popups

Storybook also allows users to change the image or text
of a popup through a GUI.

This patch sets up Storybook as a "mini" repo inside
the.storybook folder with a seperate package.json file
to avoid incompatibilities with the current webpack/babel
(or even Node) versions used in the Popups repo.

Storybook requires at least Node v8.3 to run.
(an .nvmrc file with 11.3.0 has been added to the .stories dir).

To start:
`cd .storybook && npm install && npm run start`.

Bug: T205989
Change-Id: I041e46c4f0cf173950015067e2dce81c023d3fdd
2019-01-08 14:19:00 +01:00
Stephen Niedzielski 81a2f4acb0 Hygiene: copy MobileFrontend Webpack learnings
Copy learnings from MobileFrontend's Webpack configuration. If nothing
else, the files are more consistent and easier to diff. When the change
to rename source maps is excluded, the build products are identical.

The following changes were copied:

- DRY up the output directory and source map extension as variables. For
  the latter, rename the source map from ".json" to ".map.json". Without
  renaming, the build products are unchanged.
- Reduce verbosity. Only report warnings and errors.
- Fail to build when an error occurs.
- Update ordering and add comments for easier reasoning and diffing with
  MobileFrontend.

Bug: T212527
Change-Id: Icf11dff91358ad021932aa209c65ed8aac77d12b
2019-01-02 21:30:12 +00:00
Stephen Niedzielski 578233674d Fix: rebuild dist
Due to loosely versioned dependencies, master's resources/dist is out of
date. This won't be an issue when package-lock is supported. Rebuild the
assets as a workaround.

Change-Id: I2bb8eab5b849616f5ae96a7915f8f1a9069109a6
2018-12-20 15:30:55 +00:00
Stephen Niedzielski 4ce0246d78 Hygiene: replace jQuery.noop dependency
Replace $.noop dependency with inline empty function.

The tests relied on a single function instance to pass. A toString()
comparison of the noop function cannot be used as they differ when
coverage is enabled.

Change-Id: I641801593beb240a8f7d06e388a0e41dc8a25bc6
2018-12-19 15:52:31 +00:00
Nicholas Ray 63031429b8 Upgrade redux / redux-thunk
redux       | 3.6.0 | 4.0.1
redux-thunk | 2.2.0 | 2.3.0

Upgrading these two dependencies caused the build size to increase from
from 12.2 KB to 12.4 KB (gzip) and so the bundlesize was adjusted
accordingly in our package.json.

Additionally, our linters flagged two rules that were then turned off in
.eslintrc.es5.json.

Looks like the changes in Redux were mostly cosmetic with much work
dedicated to typescript definitiions and dropping support of private
imports [1].

The changes to redux-thunk only involved changing typescript typings and
should not affect us. [2]

The upgrade was tested locally and did not appear to break anything.

[1] https://github.com/reduxjs/redux/issues/1342#issue-130452197
[2] https://github.com/reduxjs/redux-thunk/releases/tag/v2.3.0

Bug: T209314
Change-Id: I35284793ca0c72914d9b9b2e7c28dd407bafd4d8
2018-12-13 15:04:03 -07:00
Nicholas Ray 0c1a65f39d Upgrade webpack / webpack-cli
webpack     | 4.1.1  | 4.27.1
webpack-cli | 2.0.12 | 3.1.2

The upgrade of webpack modified our build files and included these
side effects:

* Our linter flagged the usage of bitwise operators in the build files
so I turned that off in our build file lint config (.eslintrc.es5.json)

* Our build file's size increased slightly from 12.1 KB to 12.13 KB
(gzip) so the package.json's bundlesize was updated accordingly.

No deleterious effects were noticed to popups running locally, but the
jump in webpack version included these notable changes:

* Switch from uglify-es to terser minimizer [1]

[1] https://github.com/webpack/webpack/releases/tag/v4.26.0

The upgrade for webpack-cli was a major version jump, but the params
that we pass to it in our package.json file appear to work as before.

Bug: T209314
Change-Id: I1403f2c4d354cf54554a740f8c23176bf80fd3c6
2018-12-13 14:53:54 -07:00
Shreyas Minocha 22226f3367
Switch from babel-preset-env to @babel/preset-env
Replace:
    - babel-preset-env@1.6.0 with @babel/preset-env@7.2.0
    - babel-register@6.24.1 with @babel/register@7.0.0
    - babel-loader@7.1.4 with babel-loader@8.0.4

Add:
    - @babel/core@7.2.0

---

- babel-preset-env, @babel/preset-env
  babel-preset-env moved to @babel/preset-env in the Babel monorepo.
  This appears to have incremented the version from v1.6.0[0] to 7.x to
  match the rest of the Babel packages but appears to otherwise be
  undocumented.[1,3]

  [0] https://github.com/babel/babel-preset-env/blob/24b99ec/README.md
  [1] https://github.com/babel/babel/blob/efa571a/CHANGELOG.md

- @babel/preset-env, @babel/register
  *Many* changes as identified by package [2] and summarized [3].

  Node.js v6 or above now required.

  New dependency on @babel/core.

  Support for ES2018 and browserslist v4.

  `modules: false` is now the default for preset-env + babel-loader. The
  .babelrc has been updated.

  New babel-upgrade tool.

  babel.config.js can replace .babelrc. Popups doesn't seem to need it.

  TypeScript and JSX fragment support.

  [2] https://github.com/babel/babel/blob/efa571a/CHANGELOG.md
  [3] https://babeljs.io/blog/2018/08/27/7.0.0

- babel-loader
  Support for Babel 7.x.

  The following warning is printed when building but perhaps this is due
  to another dependency:

    (node:14559) DeprecationWarning: loaderUtils.parseQuery() received a
    non-string value which can be problematic, see
    https://github.com/webpack/loader-utils/issues/56 parseQuery() will
    be replaced with getOptions() in the next major version of
    loader-utils.

  https://github.com/webpack/loader-utils/issues/56#issuecomment-286117000
  https://github.com/babel/babel-loader/releases/tag/v7.1.5
  https://github.com/babel/babel-loader/releases/tag/v8.0.0-beta.0
  https://github.com/babel/babel-loader/releases/tag/v8.0.0-beta.1
  https://github.com/babel/babel-loader/releases/tag/v8.0.0-beta.2
  https://github.com/babel/babel-loader/releases/tag/v8.0.0-beta.3
  https://github.com/babel/babel-loader/releases/tag/v8.0.0-beta.4
  (v8.0.0-beta.5 was erroneous.)
  https://github.com/babel/babel-loader/releases/tag/v8.0.0-beta.6
  https://github.com/babel/babel-loader/releases/tag/v8.0.0
  https://github.com/babel/babel-loader/releases/tag/v8.0.1
  https://github.com/babel/babel-loader/releases/tag/v8.0.2
  https://github.com/babel/babel-loader/releases/tag/v8.0.3
  https://github.com/babel/babel-loader/releases/tag/v8.0.4

Bug: T197883
Change-Id: Ie3a5404630fde87ea7fe618a842950ed8c0c6494
2018-12-11 13:09:45 +05:30
Stephen Niedzielski c7e742743b Doc: fix fetch delay comments
FETCH_COMPLETE_TARGET_DELAY is used to introduce an artificial delay to
HTTP requests as needed. However, FETCH_START_DELAY is always accounted
for so it makes sense to define FETCH_COMPLETE_TARGET_DELAY with it. The
docs are updated to draw the distinction between total delay and API
response delay.

Change-Id: I4cddc89b8090d54db0dd85f270441cab17c54993
2018-10-17 16:44:16 -06:00
Stephen Niedzielski 6e3f90016a Doc: fix type in getPageviewTracker()
Change-Id: I45b9456e0869657069c4a961171fd89415bf5656
2018-10-05 10:41:23 -06:00
Fomafix 5b6fe60407 Do not use jQuery's hasClass with space separated classes
A parameter with space separated class names is not specified in
https://api.jquery.com/hasClass/

Change-Id: I1b44215115a3e51319c042f0f20547cff640ab52
2018-09-21 08:20:12 -06:00
Fomafix ae472fe689 Simplify JavaScript by using native ES5 instead of jQuery
jQuery.isFunction is deprecated since jQuery 3.3.
https://blog.jquery.com/2018/01/19/jquery-3-3-0-a-fragrant-bouquet-of-deprecations-and-is-that-a-new-feature/
A simple thruthy check suffices here.

Also make the parameters of isValid mandatory.

Change-Id: Ief595dd3304016011cf6df1ffbe88cd51d4ec9ea
2018-09-18 10:08:34 -07:00
Stephen Niedzielski 16004549cf Doc: add linkInteractionToken description
Add documentation for the Schema:Popups' linkInteractionToken property
in the EventLogging reducer.

https://meta.wikimedia.org/wiki/Schema:Popups

Bug: T203013
Change-Id: I7fc872beda284ef8639ad036ddeb9efc8581a452
2018-09-05 20:02:32 +00:00
jdlrobson 53d1a2c329 Use getPageviewToken api
Currently the Popups schema is disabled but if we were to re-enable
it, the page tokens would now be consistent with the ReadingDepth
schema

The getToken function is kept to allow generation of the
pageInteractionToken's relating to a link interaction, for which
there is no centralised API.

Additional changes:
* I've updated various tests to use the central mw.user stub to make
clearer where tokens come from.

Bug: T203013
Change-Id: If746bea5aeed2b4c192a9b8a02feb1fe06480633
2018-09-05 20:02:17 +00:00
jdlrobson 04afdd4741 Update package.json to ensure assets get built on every commit
diffs now correctly output errors. This will pick up changes to
index.js and index.js.json that have not been committed

--exit-code is passed to ensure that if there are changes then an
error is thrown and the action is not allowed.

The dist folder became out of date as depending on what version
of node you run, the output is different.

To aid debugging, the script that checks the diff now outputs the
node version and the npm version. From now on, we will all have
to use the same node version to build assets.

This is annoying, but we will re-evaluate the approach we are taking
to build assets in T202743. We can easily work around this by all using
nvm and making consistent use of the same node version.

The assets have been rebuilt with the node version that CI uses.

Bug: T202748
Change-Id: I82aee879d4b04ca06447f95eb81230bfc24d20e9
2018-09-05 13:36:39 -06:00
Stephen Niedzielski 76195013be Doc: fix typo
Change-Id: I14fc469a735d8838f093fbd0b23e8f5235b4dfc9
2018-08-14 12:42:56 -05:00
Nicholas Ray 6a25f70ad5 Use window.devicePixelRatio instead of deprecated jQuery.hidpi
jQuery.hidpi was deprecated by T127328
(https://gerrit.wikimedia.org/r/441614). This repo used the
"bracketedDevicePixelRatio" function from that plugin. Since browser
compatibility is good now for window.devicePixelRatio, this commit adds
a function which relies on that instead.

Bug: T198579
Change-Id: I56c234048d7741f12f35bfff5f7319c6e085c29f
2018-08-10 10:00:28 -06:00
jdlrobson a714eda573 Revert "Whole popup area should be clickable"
This change made it impossible to open links in new tabs.
Reverting so we can try again.

This reverts commit ff5bfd1d04.

Bug: T200940
Change-Id: I10a387df8bdeb891f8d8be0eb9075f0d324646b6
2018-08-08 21:26:07 +00:00
Volker E ed529488f8 Manually optimize & reduce SVG markup
Reduce and optimize SVG markup by
- removing unnecessary `id` attributes,
- lowercasing hex colors following code standard
- merging repeating `fill` attributes in `<g>` elements and
- following `<svg>` root element attribute sequence as anywhere else

Change-Id: I71df1df5fee0c363df41a996b5ec32d587b76f56
2018-08-08 17:09:15 +02:00
Stephen Niedzielski f96762880b Fix: viewBox in cog.svg and sad-face.svg
svgmin was removing this attribute from cog when the capitalization was
incorrect and sad-face never had it.

Bug: T185596
Change-Id: I980694f4a611ea8fd13d6c1419dbed6d49f4d94e
2018-08-08 09:19:55 -05:00
Volker E f47e333ff1 build: Bring SVGO optimization to build step
Enabling SVGO automation with 'grunt-svgmin' and conservative
plugin settings to build step, among those:
- enable removeRasterImages and sortAttrs,
- disable cleanupIDs, removeDesc, removeTitle, & removeViewBox as
  described in
  https://www.mediawiki.org/wiki/Manual:Coding_conventions/SVG#Exemplified_safe_configuration
- disable removeXMLProcInst; if the SVG doesn't start with an XML
  declaration, then it's MIME type will be detected as "text/plain"
  rather than "image/svg+xml" by libmagic and, consequently, MediaWiki's
  CSSMin CSS minifier. libmagic's default database currently requires
  that SVGs contain an XML declaration:
  <https://github.com/threatstack/libmagic/blob/master/magic/Magdir/sgml#L5>.
- make use of pretty and multipass options.

Settings are stored in a JSON file to be independent of the Grunt build
process. Also updating SVG accordingly.

Bug: T185596
Change-Id: I715ad4cf2e900665e4c32c78b4c2d9d9cebf0222
2018-08-01 10:51:43 -05:00
Stephen Niedzielski 8ba5c0f773 Hygiene: make JSDoc configs consistent
Make the Popups, MobileFrontend, and MinervaNeue JSDocs consistent. For
Popups, specify package.json, readme, and default template options and
moved doc/ to docs/ and autogenerated JavaScript documentation from
doc/autogenerated to docs/js.

http://usejsdoc.org/about-configuring-jsdoc.html
http://usejsdoc.org/about-commandline.html
http://usejsdoc.org/about-configuring-default-template.html

Bug: T188261
Change-Id: I81e64f06265f1ecc4e2ee159deef9b204ea7e957
2018-07-23 14:45:14 -05:00
jdlrobson 69efbfc377 Enforce eslinting for jsdoc
Let's improve our documentation by linting it and ensuring it
is complete and matches guidelines

This fixes offenders

Change-Id: I7c829b375705e763085cf731e9a77cc14339af67
2018-07-17 08:21:01 -05:00
Stephen Niedzielski ab7a5808ef Hygiene: update JSDoc boxed and JQuery types
Although Popups only uses JSDocs at this time which seemingly doesn't
care about casing[1], we should endeavor to use the proper return types.

This patch lowercases typing to indicate primitive / boxed type as
appropriate.[2] As a special case, function types are uppercased for
compatibility with TypeScript type checking.

Lastly, JQuery types are of type "JQuery". The global JQuery object's
identifier is "jQuery". This patch uppercases J's where appropriate.

[0] https://github.com/jsdoc3/jsdoc/issues/1046#issuecomment-126477791

[1] find src tests -iname \*.js|
    xargs -rd\\n sed -ri '
      s%\{\s*([?!])?(number|string|boolean|null|undefined)%{\1\L\2%gi;
      s%\{\s*([?!])?(function|object)%{\1\u\2%gi;
      s%\{\s*([?!])?jquery%{\1JQuery%gi
    '

Change-Id: I771bdbb69dc978796a331998c0657622ac39c449
2018-07-17 08:20:08 -05:00
Piotr Miazga c823a0e6cb When request gets aborted it shouldn't finish with FETCH_FAILED
Whe user moves mouse away and we abort the http request we shouldn't
count that request as a FETCH_FAILED. The reasoning behind is that
FETCH_FAILED state increments the counter.PagePreviewsApiFailure.
Our StatsD graph gets polluted with lots of aborted requests and it
becomes unsuable. It doesn't show only the failed requests.

Changes:
 - introduced new state: FETCH_ABORTED
 - switch to FETCH_ABORTED when browser aborts the request

Bug: T199482
Change-Id: I58047eb80f0700b78b2991daff9395ecc92553b8
2018-07-13 16:52:53 +02:00
jdlrobson 10e6e25091 Upgrade eslint-config-wikimedia
* Force arrow-parens
* Disable no-prototype-builtins for time being
* Drop unnecessary maxlen rule

Change-Id: Iceb0fe47354a5753202d2c6ad9e1a9c76791f744
2018-07-13 07:42:12 -05:00
Piotr Miazga 2527f931a2 Properly handle catch() when calling gateway fetch.
Previous implementation did not pass the `result` variable
to the catch() statement. Because of that every execution that
ended with exception inside fetch() statement was threated as
not a network exception and tried to present the null preview.

Changes:
 - properly handle data returned by rejected fetch promise
 - chaged the big if (result && result....) into something easier
 to read
 - pass Error object instead of 'http' string
 - Restbase can return exception, it doesn't have to handle the 404
 errors by itself, it's already taken care in the catch() logic
 - fixed unit tests to reflect new logic in restbase gateway

Bug: T199482
Change-Id: Ibb30fc58248623d9ad4c5388a5b2ff9b387e01de
2018-07-13 00:02:59 +02:00
Stephen Niedzielski a0dc96cac9 Hygiene: consistently refer to globals directly
Instead of mixing window.mediaWiki / mediaWiki and window.jQuery /
jQuery references, always refer to globals which exist whether code is
executed in browser or headless Node.js environments.

  find src tests -iname \*.js|
  xargs -rd\\n sed -ri 's%window.(mediaWiki|jQuery)%\1%gi'

Change-Id: I21d0a602dcbd2bc6774934bee6c487e443270fe0
2018-07-09 08:46:40 -05:00
Piotr Miazga c1d281f0dc Send the Accept-Language header when calling API
Changes:
 - added acceptLanguage as a config option passed to
 both mwApi and restbaseApi, by default code will use
 the language defined in `wgContentLanguage` config
 variable. The `wgContentLanguage` is always defined
 (see ResourceLoaderStartUpModule::getConfigSettings())
 so there is no need for checking the variable existence.

The new logic was tested both on MediaWiki API and Restbase API

Bug: T198619
Change-Id: I1cb31f1999fd674a8b870b2b5effb92ed3dfaa1f
2018-07-05 11:31:55 -07:00
Stephen Niedzielski ce8a2d4c3f Update: cancel unused HTTP requests in flight
Whenever an HTTP request sequence is started, i.e. wait for the fetch
start time, issue a network request, and return the result, abort the
process if the results are known to no longer be needed. This occurs
when a user has dwelt upon one link and then abandoned it either during
the fetch start wait time or during the fetch network request itself.

This change is accomplished by preserving the pending promises in two
actions, LINK_DWELL and FETCH_START, and whenever the ABANDON_START
action is issued, it now aborts any previously pending XHR-like promise,
called a "AbortPromise" which is just a thenable with an abort() method.
There is a similar concept in Core:
ecc812f06e/resources/src/mediawiki.api/index.js.

Aborting pending requests has big implications for client and server
logging as requests are quickly canceled, especially on slower
connections. These differences can be observed on the network tab of
DevTools and the log in Redux DevTools.

Consider, for instance, the scenario of dwelling upon and quickly
abandoning a single link prior to this patch:

  BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_END STATSV_LOGGED ABANDON_END EVENT_LOGGED FETCH_COMPLETE

And after this patch when the fetch timer is canceled (prior to an
actual network request):

  BOOT EVENT_LOGGED LINK_DWELL ABANDON_START ABANDON_END EVENT_LOGGED

In the above sequence, FETCH_* and STATSV_LOGGED actions never occur.

And after this patch when the network request itself is canceled:

  BOOT EVENT_LOGGED LINK_DWELL FETCH_START ABANDON_START FETCH_FAILED STATSV_LOGGED FETCH_COMPLETE ABANDON_END EVENT_LOGGED

FETCH_FAILED occurs intentionally, STATSV_LOGGED and FETCH_COMPLETE
still happen even though the fetch didn't complete successfully, and
FETCH_END doesn't.

Additionally, since less data is transmitted, it's possible that the
timing and success rate of logging will improve on low bandwidth
connections.

Also, this patch tries to revise the JSDocs where possible to support
type checking and fix a call to the missing assert.fail() function in
changeListener.test.js.

Bug: T197700
Change-Id: I9a73b3086fc8fb0edd897a347b5497d5362e20ef
2018-07-04 13:48:14 -05:00
Stephen Niedzielski 89e592183f Hygiene: enable Redux DevTools for debug builds
Redux DevTools are available in all builds by passing the `?debug=true`
query string. Since globally enabling debug significantly slows load
times, also enable support when the build is non-production (debug)
which is known at transpile time. This enables a debuggable version of
Popups in an otherwise production-like MediaWiki without changing the
Popups release build product.

Also, update the readme with a couple debug tips and flip a few bullets
from hyphens to asterisks since that seems to be more prevalent.

Change-Id: I4cab0b8069b12505dbfa840939caac196bae2750
2018-07-02 14:54:36 -05:00
jdlrobson 6c17af260c Extracts can expand with narrow thumbnails
If a thumbnail is narrow, then the extract can expand to take
the available space. It does this via JavaScript taking the difference
between the normal space for a thumbnail minus the actual space needed
to display the thumbnail.

This removes unused whitespace in both the thumbnail and extract.

Bug: T192928
Change-Id: I59e87f9160e707fbce321a567c0a68e85f6d72ec
2018-06-28 16:34:41 -07:00
Jan Drewniak 4e43f0cf9e Truncate source_url to max 1000 characters
Prevents the source_url param in virtual page-views from getting
too long and causing an error because it exceeds varnish's max-url size.

Bug: T196904

Change-Id: Idf3667c4c2ad7e0436f013c70d5ff4ebea453d7a
2018-06-28 14:30:42 -07:00
jdlrobson ff5bfd1d04 Whole popup area should be clickable
Make it so the entire popup area is clickable.
Update the click handler to reflect the actual parameter
it receives (an Event not an Element) and do not pass it
in the action, given it is unusedt

Bug: T192773
Change-Id: If80969f4759b1675278d11caaf5cb093ce72031c
2018-06-28 20:50:16 +00:00
jdlrobson 21c8e6213e Add SVG border using polyline element
Since we use an SVG mask, we cannot use border-left to visually
separate the page preview thumbnail from the text. We can however
make use of a polyline and programatically work out it's start and
end.

Bug: T192928
Change-Id: I0f983a80e3210b2f7e9aa197d2a632680675973e
2018-06-28 11:23:48 -07:00
Jan Drewniak 3b2480d6ce Ensure popup thumbnail images are a supported format
Prevents video files and other non-image files from being rendered as
popup thumbnails. Restricts thumbnail format to either jpg, png, or gif.

Bug: T193792
Change-Id: I7a9be5d1c8396c02ebf0893c960f65644acc9d99
2018-06-21 00:55:04 +02:00
Stephen Niedzielski abc2026890 Hygiene: replace QUnit assert.equal with strictEqual()
Via jscodeshift:

  jscodeshift \
    -t jscodeshift-recipes/src/qunit-assert-equal-to-strictEqual.js \
    Popups/tests

Also, some very minor manual clean up.

https://github.com/niedzielski/jscodeshift-recipes/blob/5944e50/src/qunit-assert-equal-to-strictEqual.js

Additional change:
* Drop redundant clipPath parameter from createThumbnailElement - this
parameter does not exist in the function signature.

Change-Id: I209ecf2d54b6f5c17767aa2041d8f11cb368a9b5
2018-06-18 19:48:16 +00:00
Stephen Niedzielski 3e248d75cc Hygiene: refactor common popup template code
Move the outer container common to all previews to a new template.

Bug: T191646
Change-Id: I8f3d99b25c457495ece7b66bfa6026fe827608be
2018-06-14 07:50:22 -05:00
Jan Drewniak a782b7b7da Update setting icon
- Adds an extra element in popups template to contain settings icon.
- Resizes the setting icon so that the hover/active state is a square.
- Updates the settings icon SVG file.
- Modifies margin rule for icons placed near top of popup.

Bug: T193058
Change-Id: Icc16a788bba8e2f0a82a27c2b5c7be6c2cccaa90
2018-06-05 10:07:35 +01:00
jdlrobson 79daacb235 Coalesce and cleanup use of then blocks
Bug: T190141
Change-Id: I345befc24397e27c965af0a593821333f2a71f21
2018-05-30 17:32:08 +01:00
Stephen Niedzielski ae44042cbf Hygiene: add assertion messages
Change-Id: Ic0a47bd468532824e8648c3f6371cc403896603c
2018-05-08 15:55:23 -05:00
jdlrobson 912402e840 Remove A/B testing code
No longer needed. We can't turn something off again for people
now they expect it to exist.

Clarified usage instructions of PopupsEventLogging to make sure
it's more scary given the implications

Bug: T173952
Change-Id: I7be005b79da498d8e7b7df8f18b60c1327636a2c
2018-05-07 12:37:41 -07:00
jdlrobson 4e3282e5ff Remove BetaFeature code
Popups is out of beta feature and this code is no longer needed.
Removing code is the happiest activity a developer can do.

Other changes:
* Remove redundant type field on extension.json
(If not set, the extension will default to the "other" section.)
* Repurpose `name` with `namemsg` and make use of existing i18n
messages

Bug: T193053
Change-Id: Iea832cd1f37b0e7df6ff95efd66e4a1ff2a9004e
2018-04-26 15:51:48 -07:00
Stephen Niedzielski d7871bb9c4 Hygiene: replace okies with ointers
I've yet to meet the bloke who knew how to take a poke without an
explanation such that they have never mispoke. This patch which renames
pokies to pointers will surely be my masterstroke.

  find \
		-not \( \( -name node_modules -o -name .git -o -name vendor -o -name dist -o -name package-lock.json \) -prune \) \
		-type f|
	xargs -rd\\n sed -ri 's%([Pp])(okey|okie)%\1ointer%g; s%([Pp])oke%\1oint%g'

Bug: T190831
Change-Id: I363e6dd49bfcdb9515cd5fab2904a58725b18720
2018-04-26 13:26:48 -07:00
Stephen Niedzielski 7c98c04e0b Fix: unwanted thumbnail spacing in RTL locales
Thumbnails are displayed as SVG image elements. The SVG itself has a
width 3px greater than necessary for landscape thumbnails specifically.
For left-to-right languages, this additional space is empty and
unnoticed. For right-to-left languages, this extra spaces shows as a gap
on one side of the thumbnail and exceeds the popup's bounds on the other
side.

This extra 3px appears to have been mistakenly applied to landscape
thumbnails when it is only applicable to portrait, for which it is
already accounted for. Remove the 3px slop.

Bug: T190831
Change-Id: I6096f416f7e102975c4753a6b093b192aa1b45d7
2018-04-26 08:59:00 -05:00
Stephen Niedzielski a4e129175a Fix: show thumbnails on left for right-to-left UIs
When the UI is RTL, show preview thumbnails on the left. Otherwise, show
them on the right.

Bug: T190831
Change-Id: Ic1fc54f6547b31908905db8cb2ec4d58f37a6538
2018-04-23 16:59:19 -05:00
Stephen Niedzielski 44a7f643bc Hygiene: replace CSS class underscores w/ hyphens
Replace CSS slithery_snake_case with shish-kebab-case for consistency
with the rest of the codebase.

  find \
    -not '( (
      -name node_modules
      -o -name .git
      -o -name vendor
      -o -name doc
      -o -name dist
      ) -prune
    )' \
    -type f|
  xargs \
    -rd\\n \
    sed -ri 's%flipped_([xy])_([xy])%flipped-\1-\2%g; s%flipped_([xy])%flipped-\1%g'

Change-Id: I25dc0ddeda711faca9a79b5bf87d6b5aa0d5aea5
2018-04-23 16:23:17 -05:00
Stephen Niedzielski 3a372ac3ab Hygiene: rename triangle terminology to pokey
Everyone knows what a poke is.

  find \
    -not '( (
      -name node_modules
      -o -name .git
      -o -name vendor
      -o -name doc
      -o -name dist
      ) -prune
    )' -type f|
  xargs -rd\\n sed -ri 's%\btri(\b|angle)%pokey%g'

Change-Id: Ie159aa6947801a98cbf358da1613c87cf66d548f
2018-04-23 16:07:51 -05:00
Stephen Niedzielski 676faa3514 Hygiene: consolidate CSS class manipulation
• Instead of removing 'mwe-popups-no-image-tri' in
  renderer#layoutPreview(), add more conditions to #getClasses().

  The addition condition in getClasses() was:

    ( !hasThumbnail || isTall ) && !flippedY

  The removal condition in layoutPreview() was:

    flippedX && !flippedY && hasThumbnail && isTall

  To combine them, the removal logic is inverted and the conjunction is
  taken:

    ( ( !hasThumbnail || isTall ) && !flippedY ) &&
    !( flippedX && !flippedY && hasThumbnail && isTall )

  Push the negation inwards:

    ( !hasThumbnail && !flippedY || isTall && !flippedY ) &&
    ( !flippedX || flippedY || !hasThumbnail || !isTall )

  Expand:

    !hasThumbnail && !flippedY && !flippedX ||
    !hasThumbnail && !flippedY && flippedY ||
    !hasThumbnail && !flippedY && !hasThumbnail ||
    !hasThumbnail && !flippedY && !isTall ||
    isTall && !flippedY && !flippedX ||
    isTall && !flippedY && flippedY ||
    isTall && !flippedY && !hasThumbnail ||
    isTall && !flippedY && !isTall

  Eliminate always false conditions and combine redundancies:

    !hasThumbnail && !flippedY && !flippedX ||
    !hasThumbnail && !flippedY ||
    !hasThumbnail && !flippedY && !isTall ||
    isTall && !flippedY && !flippedX ||
    isTall && !flippedY && !hasThumbnail

  Further eliminate redundancies:

    !hasThumbnail && !flippedY ||
    isTall && !flippedY && !flippedX ||
    isTall && !flippedY && !hasThumbnail

  Factor:

    !flippedY && (
      !hasThumbnail || isTall && !flippedX || isTall && !hasThumbnail
    )

  Factor more:

    !flippedY && (
      !hasThumbnail || isTall && ( !flippedX || !hasThumbnail )
    )

  Eliminate last redundancies:

    !flippedY && ( !hasThumbnail || isTall && !flippedX )

  The getClasses() test is updated for the new logic.

• Move thumbnail clip path manipulation from renderer#layoutPreview() to
  a new function, #setThumbnailClipPath(). The new function flips the
  order of the series of if statements so that an if / else block can be
  used instead which clarifies that clip-path state is exclusive. In
  other words:

    if ( a ) { foo.prop = 1; }
    if ( b ) { foo.prop = 2; }
    if ( c ) { foo.prop = 3; }
    if ( d ) { foo.prop = 4; }

  Can generically be refactored regardless of condition or value to:

    if ( d ) { foo.prop = 4; }
    else if ( c ) { foo.prop = 3; }
    else if ( b ) { foo.prop = 2; }
    else if ( a ) { foo.prop = 1; }

  Because prop was originally overwritten which implies if / else-like
  priority.

  Additionally:
  • The entire function call is wrapped in a hasThumbnail conditional
    which previously was checked as an input in each case.
  • Consolidate the last two conditions since they only differed by a
    single boolean input.
  • Move the setAttribute() action to the end of the function since the
    conditionals just map condition to value and the action is now
    identical.

• Revise pokey mask doc to use clip-path terminology. This inverts the
  thinking about the mask but better matches usage.

Bug: T190831
Change-Id: Ib460c6c07fcb054f8d425d127c588bb28a1d2473
2018-04-23 15:56:25 -05:00
Piotr Miazga 321d6348e1 Page_title and source_title should be in canonical form
We change page_title and source_title as a last step
just before we send the event.

Doing it elsewhere is risky at the current time because:
* the non-canonical form is needed for
mwApiGateway to bold title (formatPlainTextExtract). Needing both
would require updates to the Page model.
* title is used by EventLogging (Schema:Popups)

Bug: T191471
Change-Id: I93e7343643dcd9f32a86459907eb0b7051df91aa
2018-04-16 19:56:43 +02:00
jdlrobson 44b49ab163 Drop second requestIdleCallback call
We already request an idle callback when loading the code in the init
module.
Doing so here seems redundant.

Bug: T191089
Change-Id: If132c3331c49a4e74be70a5486a8270a8ce380bd
2018-04-16 12:40:29 +02:00
jdlrobson 1c2908590f Speed up the time to page preview hovers
...by using a delegate event handler on the document instead of waiting
in the hook for the content div.

Bug: T191089
Change-Id: I88baa12a9aad9ed5e6c1288b39089843c19cec6c
2018-04-16 12:35:35 +02:00
Stephen Niedzielski 2eeaa0a2e4 Hygiene: consolidate clip-path manipulation
The clip-path SVG property was conditionally set in thumbnail.js and
also conditionally set or removed in renderer.js. This patch refactors
the logic to occur in a single place, renderer.js.

The refactor was made with the following considerations:

• The one condition under which thumbnail.js would set clip-path was,
  given a thumbnail exists, the thumbnail was not tall and clip-path
  would be set to `url(#mwe-popups-mask)`. Otherwise, thumbnail.js would
  not set clip-path.

• The logic in renderer.js for setting the attribute doesn't change
  since overwriting the clip-path is equivalent to not having a
  preexisting value. The case for removing the attribute itself is
  replaced by inverting the condition, `flippedY`, and combining it with
  the thumbnail.js condition, `!isTall`. The operation is only valid for
  an existing thumbnail so the `hasThumbnail` remains unchanged.

This patch also clarifies that the "flipped" classes are exclusively set
by using an if / else chain instead of reconsidering all inputs for each
condition.

Bug: T190831
Change-Id: I4062ec7068dcadecbdbc4791447ea2ed1ce2a1de
2018-04-13 09:25:22 -05:00
Stephen Niedzielski c61513f0ea Hygiene: split LESS into components
Bug: T165036
Change-Id: I916f27138b5abe6b9fe23719cffb10f20e604b89
2018-04-03 16:37:05 -07:00
jdlrobson 663218d974 All images are served by ResourceLoaderImageModule
This moves the footer icon into the ResourceLoaderImage module
providing us a consistent way of serving image assets.

This means we no longer need to provide PNGs for icons

However, given mw-ui-icon-large is not large enough for the given
use case we do have to wrestle with icon styles and override them
to get the desired result. I think this is a small price to pay given
icons are now discoverable

Change-Id: I38b62c01fd930dcbfb73b95e6128885cb483f86e
2018-04-03 16:34:49 -07:00
jdlrobson e6202b6ce4 VirtualPageViews bypass EventLogging for logging virtual events
We are using EventLogging to track page views not user behaviour.
This is an exception to the rule and requires special handling.

Bug: T190188
Change-Id: If096ccaf0ac884d57744ed57e2f26b51446de2d7
2018-03-29 16:53:20 +00:00
jdlrobson 1acd15adb1 Standardise disambiguation icon
All icons should be 20 by 20 by standard.
(Previously it was 24 by 24)

Bug: T190558
Change-Id: I0956c9c31f1606f0a77b4be29bde5a56b5c5ebb8
2018-03-23 13:54:31 -07:00
jdlrobson 90b787ed9c Abandon use of skinStyles for settings dialog
We already use pixels to define font sizes in the pixels so rather
fight Vector's styles explicitly declare the font size in the header
(which it controls) instead for more readable code.

Also copy across several rules that are implied by Vector but needed
for the settings dialog so that this renders nicely in Monobook and
Minerva.

Change-Id: I196b8af23fe6ebcdb472c059cf1536d251a5b978
2018-03-23 11:13:00 -07:00
joakin b18ea67ea1 Hygiene: Update comment of application initialization
Comment became a bit stale. This updates it to reflect reality and
normalizes the verb forms for consistency.

Noticed as part of https://gerrit.wikimedia.org/r/c/420839/2/src/index.js#167

Additional changes:
* Merged all the const statements in L63 into one, for consistency.
  Before, some were merged and some were independent.

Change-Id: I23aa824bb811f03a3630b4695e84c468bd9cd8b5
2018-03-23 09:32:52 -05:00
Stephen Niedzielski 4dd9df12eb Doc: update some of the popups / preview terminology
Bug: T165036
Change-Id: Id75543d4d886e4cca8278baaf4f4a21c8e4389b7
2018-03-23 11:18:26 +01:00
Stephen Niedzielski c4b50b04ac Hygiene: remove unused resources
In I7395e3438836149becdd576942bdaf6f21b4163f the settings templates
were rewritten so that they no longer displayed an image.

descriptionText and images were dropped from the template but not from
the template provider. These are artifacts from relating to that patch
and are no longer used.

Change-Id: I1be7ef288d37f338e83dab3cf041e628a06608d2
2018-03-21 17:07:18 -05:00
Stephen Niedzielski f974fc4f3c Fix: use localized close labels in settings dialog
Use i18n close label in the settings dialog since we have it instead of
hardcoding English. A cross is shown instead of the text so this change
may not be visible in all browsers.

Change-Id: I66284b04fe905cc8e460ea10b56c88cacd66ed28
2018-03-21 21:50:47 +00:00
jdlrobson 3245a2ac6e Hygiene: restrict use of $.each and fix offenders
Bug: T190142
Change-Id: I5da7a4a1ffe14647ec70b10b3a7ab9b935c5ca84
2018-03-21 14:24:22 -05:00
Stephen Niedzielski 007cc5ab7c Hygiene: favor string templates over concatenation
Update the ESLint config to favor ES6 strings where variables or string
concatenation is used and fix *all* offenders automatically via:

  eslint \
    --cache \
    --max-warnings 0 \
    --report-unused-disable-directives \
    --fix \
    src tests

Change-Id: I1a9345162348e5ded21d541e7a2ce251ea72ab5a
2018-03-21 08:05:55 -05:00
Stephen Niedzielski 57762e0417 Hygiene: favor const
Bug: T165036
Change-Id: I17d374eaac6627ca6a8ba178862b2a9cff2538c0
2018-03-21 10:44:24 +00:00
Stephen Niedzielski 0bee0906d4 Hygiene: replace var with let and const
eslint \
    --cache \
    --max-warnings 0 \
    --report-unused-disable-directives \
    --fix \
    src tests

Change-Id: I051275126ae7fa9affd16c2504017c0584f2d9c7
2018-03-20 14:14:02 -05:00
Stephen Niedzielski ece4670710 Hygiene: use arrow for anonymous functions
In some places, the arrow function seems more natural. This patch
approximates the following with manual amendments:

  find \
    -not \( \( -name node_modules -o -name .git -o -name vendor -o -name doc -o -name resources \) -prune \) \
    -iname \*.js|
  xargs -rd\\n sed -ri 's%function\s*(\([^)]*\))%\1 =>%g'

Files to focus on were identified with:

  rg -slg\!/resources/dist/ -g\!/i18n/ -g\!/doc/ 'this|self|arguments|bind|call|apply|new'|
  xargs -rd\\n git difftool -y

Bug: T165036
Change-Id: Ic66b6000b8fc000f9bfde39749f9cfa69924a13c
2018-03-20 09:27:08 -05:00
Stephen Niedzielski a67466acc0 Hygiene: replace obvious function methods
Replace easily identifiable object functions with method syntax:

  find \
    -not \( \( -name node_modules -o -name .git -o -name vendor -o -name doc -o -name resources \) -prune \) \
    -iname \*.js|
  xargs -rd\\n sed -ri 's%:\s*function\s*(\([^)]*\))%\1%g'

Bug: T165036
Change-Id: I90693ee99a6a36dff820dd5ae6f6000429763058
2018-03-20 09:27:07 -05:00
Stephen Niedzielski a2a743d775 Hygiene: use object shorthand where obvious
Approximately:

  find \
    -not \( \( -name node_modules -o -name .git -o -name vendor -o -name doc -o -name resources \) -prune \) \
    -iname \*.js|
  xargs -rd\\n sed -ri 's%(\b\S+\b)\s*:\s*\b\1\b%\1%g'

Bug: T165036
Change-Id: I48869dc93b66f908e070288eb2f035bb064993e3
2018-03-20 09:26:20 -05:00
Stephen Niedzielski 42816702eb Hygiene: replace Mustache templates w/ ES6 strings
Replace Mustache.js templates with template literals. An effort was made
to minimize additional refactoring, so feel free to ask for more but it
ain't coming in this PS.

Bug: T165036
Change-Id: I4a6a1d93a2922c3a9ef3ae93c47da17a35c644f0
2018-03-20 08:06:02 -05:00
Stephen Niedzielski e5df865d51 Hygiene: enable Babel transpilation
Enable the Babel transpiler so that ES6 template literals,
destructuring, and arrow functions can be used in production.
"last n versions" syntax was not used so that builds are more
reproducible.

Bug: T165036
Change-Id: I553b6d14cc368c7b4366f68d13038c3d505f5429
2018-03-20 07:59:14 -05:00
Stephen Niedzielski d35286a064 Update: upgrade to Webpack 4 & improve output size
Upgrade from Webpack v2.0.12 to v4.1.1. There have been many changes to
Webpack... Noticeable in the bundle size is that a number of files are
now inlined and generally minification is improved.

Changes related to the upgrade include:

- Remove unsupported UglifyJS options which link back to T188081. These
  appear to have been anticipatory but never acted on and it isn't clear
  whether they even worked or not.
- Pass the -p plag for production optimizations; pass the -d flag since
  production doesn't support watching.
- NamedModulesPlugin is now on by default but only in development mode
  so no change was made.
- Webpack command line client now must be installed separated and
  appears in package.json as webpack-cli.

https://github.com/webpack/webpack/releases/tag/v2.2.0
https://github.com/webpack/webpack/releases/tag/v2.2.1
https://github.com/webpack/webpack/releases/tag/v2.3.0
https://github.com/webpack/webpack/releases/tag/v2.3.1
https://github.com/webpack/webpack/releases/tag/v2.3.2
https://github.com/webpack/webpack/releases/tag/v2.3.3
https://github.com/webpack/webpack/releases/tag/v2.4.0
https://github.com/webpack/webpack/releases/tag/v2.4.1
https://github.com/webpack/webpack/releases/tag/v2.5.0
https://github.com/webpack/webpack/releases/tag/v2.5.1
https://github.com/webpack/webpack/releases/tag/v2.6.0
https://github.com/webpack/webpack/releases/tag/v2.6.1
https://github.com/webpack/webpack/releases/tag/v3.0.0
https://github.com/webpack/webpack/releases/tag/v3.1.0
https://github.com/webpack/webpack/releases/tag/v3.2.0
https://github.com/webpack/webpack/releases/tag/v3.3.0
https://github.com/webpack/webpack/releases/tag/v3.4.0
https://github.com/webpack/webpack/releases/tag/v3.4.1
https://github.com/webpack/webpack/releases/tag/v3.5.0
https://github.com/webpack/webpack/releases/tag/v3.5.1
https://github.com/webpack/webpack/releases/tag/v3.5.2
https://github.com/webpack/webpack/releases/tag/v3.5.3
https://github.com/webpack/webpack/releases/tag/v3.5.4
https://github.com/webpack/webpack/releases/tag/v3.5.5
https://github.com/webpack/webpack/releases/tag/v3.5.6
https://github.com/webpack/webpack/releases/tag/v3.6.0
https://github.com/webpack/webpack/releases/tag/v3.7.0
https://github.com/webpack/webpack/releases/tag/v3.7.1
https://github.com/webpack/webpack/releases/tag/v3.8.0
https://github.com/webpack/webpack/releases/tag/v3.8.1
https://github.com/webpack/webpack/releases/tag/v3.9.0
https://github.com/webpack/webpack/releases/tag/v3.9.1
https://github.com/webpack/webpack/releases/tag/v3.10.0
https://github.com/webpack/webpack/releases/tag/v3.11.0
https://github.com/webpack/webpack/releases/tag/v4.0.0
https://medium.com/webpack/webpack-4-released-today-6cdb994702d4
https://github.com/webpack/webpack/releases/tag/v4.0.1
https://github.com/webpack/webpack/releases/tag/v4.1.0
https://github.com/webpack/webpack/releases/tag/v4.1.1

Bug: T165036
Change-Id: Ic146e680d368fee7ee207b484460ce94f8bd7283
2018-03-20 07:55:37 -05:00
jdlrobson ef045be304 Disambiguation icon should be regular mw-ui-icon size
Bug: T168392
Change-Id: I54125fdf6649f33f5a1c797b618f558b9330b62c
2018-03-19 23:23:34 +01:00
Stephen Niedzielski 674ade83e7 Hygiene: move settings dialog to separate file
Separate the settings dialog and settings dialog renderer into two
files.

Bug: T165036
Change-Id: I500add4b89ec8c5d1a76d7c5572f62204d61f62d
2018-03-14 21:39:39 +00:00
Stephen Niedzielski e26c12db52 Hygiene: move thumbnail code to separate file
Move thumbnail code from renderer to a separate file, thumbnail.

Bug: T165036
Change-Id: I6c55750ec302de6341e8e91dee34581ee66499d7
2018-03-14 14:19:41 -05:00
Stephen Niedzielski b8dfad87e8 Doc: add pokey mask documentation
Bug: T165036
Change-Id: Ib16cac249a49954b11b39bef2c1449552e3f1c01
2018-03-14 14:19:16 -05:00
Stephen Niedzielski 4eb9c0efa8 Hygiene: move SVG string to file
- Add SVG Inline Loader for Webpack. This allows SVG files to be
  imported.

- Update the Webpack and test configurations to use the new loader.

- Scope the ESLint rules down to just JavaScript files so that linting
  isn't attempted on the SVG.

Bug: T165036
Change-Id: I00bccff4c3167975c19d577be6343dcaca7ddb2d
2018-03-14 12:04:28 -07:00
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
joakin 7bd29bb058 Enable production settings for the production bundle
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
2017-05-11 10:37:45 +02:00
Sam Smith c44fddf8cd eventLogging: Discard events with duplicate tokens
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
2017-05-10 11:43:05 +01:00
Baha d6424cb59d QA: Test renderer#show
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
2017-05-09 17:15:01 -04:00
Baha 108f205e12 QA: Test renderer#layoutPreview
Bug: T133022
Change-Id: I7b7aa544b850e5487a621c522b28653db6056633
2017-05-09 17:13:16 -04:00
Sam Smith 2d3f5333b5 eventLogging: Round perceivedWait property
The perceivedWait property, introduced in I3fd253f1, must be an
integer.

Bug: T164256
Change-Id: If19e8694e5ac111af0f27146cf4ac03bd1a6ff82
2017-05-09 19:00:41 +01:00
Sam Smith 35cfc38b0b eventLogging: Add missing perceivedWait property
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
2017-05-09 06:51:05 +01:00
Baha 6d95bbf630 Refactor and test renderer#createLayout
* Make function dependencies explicit;
* Only pass data that's needed, no more.

Bug: T133022
Change-Id: Ia973bd9636a477a76db907a37e2a0689daf4e3fa
2017-05-05 14:20:46 -04:00
Ed Sanders 81a6d49e99 build: Update eslint to 0.4.0 and make pass
Change-Id: I7d301049f0c91f79e82f996b2dce770840c27e72
2017-05-05 05:43:16 +01:00
Baha 0b169321d8 QA: Test renderer#bindBehavior
Bug: T133022
Change-Id: I3b0b199010f0460d83f80c57e4bab8e3606a4b4f
2017-05-04 15:48:10 -04:00
Sam Smith 742f341e5c Hygiene: Tidy up QUnit references
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
2017-05-04 15:53:44 +01:00
Baha d95badc614 QA: Test renderer#hide
Bug: T133022
Change-Id: I752c4266b6be1909a3265d8292e7c5229e5724fb
2017-05-03 13:26:19 -04:00
Baha eb9ceb6238 QA: Test renderer#createPreview
Bug: T133022
Change-Id: Ied8e798851d5aebf2910aba5866f8a801c20923b
2017-05-03 13:23:43 -04:00
Baha a46a5fc02f QA: Test renderer#createEmptyPreview
Bug: T133022
Change-Id: If93d2c679bd7edcf98946e1cb688bc5c3744f69b
2017-05-03 13:21:24 -04:00
Baha ccca8b7a90 QA: Test renderer#createThumbnailElement
Bug: T133022
Change-Id: I7909c208e3d69b2ee510dcd8bfbcafc0118397ca
2017-05-03 16:25:28 +00:00
Baha 7f89341cc6 QA: Test renderer#getClasses
Bug: T133022
Change-Id: If3b8888a948f6e14d8847fdc1d4ea16e42329bb8
2017-05-03 16:24:18 +00:00
Baha 872691e293 QA: Bring back renderer#renderExtract tests
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
2017-05-02 12:09:11 -04:00
Baha 234282d1a2 QA: Test renderer#createPokeyMasks
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
2017-05-01 11:00:42 -04:00
Baha 2030cb6b06 QA: Bring back renderer#getClosestYPosition tests
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
2017-05-01 10:56:41 -04:00
joakin 1da916ee73 Tests: Refactor processlinks test
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
2017-04-28 22:39:49 +01:00
jenkins-bot 1adcc83af8 Merge "Track and discard duplicate enqueued events" 2017-04-28 20:06:13 +00:00
jenkins-bot 5e10affd8f Merge "Remove isLoggingEnabled with Null Object pattern" 2017-04-28 19:46:16 +00:00
Sam Smith 79f3b318d0 Track and discard duplicate enqueued events
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
2017-04-28 15:12:40 +01:00
Sam Smith 3d0899c0a0 Remove isLoggingEnabled with Null Object pattern
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
2017-04-27 14:29:04 +01:00
Sam Smith 939ebe386d Optimise SVGs with svgo
... 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
2017-04-26 17:15:45 +01:00
jenkins-bot 1a8b147f97 Merge "Don't show preview if user has abandoned link" 2017-04-24 21:33:12 +00:00
jenkins-bot 1e4a95cb73 Merge "Improve illustration on disable popup" 2017-04-24 18:45:37 +00:00
Baha 764fb36017 Improve illustration on disable popup
The updated illustration makes it easier to find the footer link
to enable Previews.

Bug: T154330
Change-Id: I8b6cbe9e2f002eaff06a293643dc0f4aa1d5a641
2017-04-24 19:26:26 +01:00
Sam Smith 7f91068ecf Don't show preview if user has abandoned link
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
2017-04-24 15:35:07 +01:00
jdlrobson b0e059b9de Generate cog icon via ResourceLoaderImage module
* 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
2017-04-21 16:23:21 -07:00
Sam Smith 5c5872d31f Don't occlude link when preview is above mouse
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
2017-04-20 16:41:19 +01:00