Commit graph

279 commits

Author SHA1 Message Date
Baha a49cdb37cc Use Schema revision 15777589 for logging events
The schema introduces a new field called `hovercardsSuppressedByGadget`,
which is `true` when the Navigation Popups gadget, rather than the Popups
extensions, is being used to fetch and display article summary.

Note that it may take some time before the Navigation Popups gadget is
loaded onto the page, thus actions taking place before this happens will
record the value of `hovercardsSuppressedByGadget` as `false`. This is
mostly the case with the `pageLoaded` action which almost always happens
before the gadget code is loaded.

Bug: T137203
Change-Id: Ie31deea7ae2323d6a346c67ed84fdf587ad55bd1
2016-07-19 16:00:41 +05:00
jenkins-bot 787f0d2179 Merge "Tweaks to settings page" 2016-07-08 05:19:24 +00:00
jdlrobson c1cb976ba2 Tweaks to settings page
* Change labels in English for settings options
* Update description for enable option
* Remove description for disable option

Bug: T138233
Change-Id: Id23dcc7b7e655f7939bb2e455b8680ed5a2c6331
2016-07-07 10:02:08 -07:00
Baha 1bdd8e7a7c Do not log XHR cancellations
These errors do not have any value, they just happen when the user
cancels the current request. Removing these events will result in
less noise for data analysis.

Bug: T137059
Change-Id: Ia9d921553791d1b7be5941a98716297b74b706b2
2016-07-07 18:47:54 +05:00
jenkins-bot 76fada71ce Merge "Workaround for display of SVG images in Safari" 2016-06-29 18:52:06 +00:00
jdlrobson 7d51ce9bb4 Workaround for display of SVG images in Safari
Tested in IE9 and Safari. It seems in Safari the mixture of jQuery
and setAttributeNS causes issues.

Bug: T138430
Change-Id: I4bc63da18d008487d0c8f7b906688e4c8c809efd
2016-06-29 17:59:20 +02:00
Volker E bc62902f4f Align Hovercards' Less to Coding Standards
Aligning Hovercards' Less to Coding Standards and also variablize
`linkpreview-title` font-family.

Change-Id: I11f2d71ce50dcd0fe47f3c5c528779e29a81cbc6
2016-06-24 16:23:16 +02:00
Baha 827e1dbeb2 Detect whether NavPopups gadget is enabled before showing a hovercard
Before this change we only used to check whether NavPopups gadget is
enabled once per page load. The premise was that we could figure out
whether the gadget was enabled easily given the module name. Since
the gadget module name can be different on diffirent wikis, that
solution would not scale.

With this change we check whether the gadget is enabled before each
hover over an eligible link. We rely on the existence of the global
`window.pg` object. The existence of the object means that the
gadget is enabled, thus we do not show Hovercards.

Bug: T135628
Change-Id: Ica154dd3bfd913202a8b558ea4b10ad177176f83
2016-06-07 17:39:58 -04:00
jdlrobson 7975fde745 Remove the need for global mw.popups.triggers
Change-Id: Iec1dd93a83fe393bd8717884ab4bab692ad7b6f4
2016-06-06 09:30:54 -04:00
jenkins-bot 9f91cd6d3c Merge "Correctly log the 'dismissed' action when hovering over another link" 2016-06-03 11:08:02 +00:00
Baha 049ae039a7 Do not show Hovercards when NavPopups gadget is enabled on huwiki
This is temporary fix to get Hovercards not to show up when
Navigation Popups gadget is enabled on huwiki. The gadget module is
named 'ext.gadget.latszer' and not 'ext.gadget.Navigation_popups'.

Also check whether the global 'pg' variable is created, which means
the Navigation Popups gadget is enabled.

Bug: T135630
Change-Id: I35e1b911967200bfdfd8f44ad4e4b8dcfd844ee7
2016-06-02 17:57:04 -04:00
Baha a8e8ca489f Correctly log the 'dismissed' action when hovering over another link
Unlike what was previously implemented, this patch does not care whether
the user hovers over another link while dismissing the popup.
The `dismissed` action is correctly logged in these cases too.

Bug: T136649
Change-Id: I68473cb8b66bae53213bce186345ca1ce436573f
2016-06-02 15:24:31 -04:00
jdlrobson dedb61caf9 Drop support for non-SVG browsers
According to caniuse.com SVG support is available
from IE > 8, Firefox > 3, Safari > 3.1 and Android
> 2.3. Android 3-4.3 does not support masking.

Out of all these browsers, considering market share
and ResourceLoader support, none of these browsers
are of concern to us. In IE8 for example we do not
run JavaScript for our end users. Thus we should remove
this fallback support.

Changes
* Remove createImgThumbnail method and its test
* Groups duplicate CSS groups
* Refactor createThumbnail function
** Leave a FIXME on some curious code

Change-Id: I59ac2e320b2e07815bc4136d5942016fdc1d4340
Bug: T135554
2016-05-31 15:23:07 -07:00
jdlrobson 6178781c43 Do not unnecessarily expose private variables
article.surveyLink, article.SIZES and currentRequest do not need to be
globally available on mw.popups.render object. They can be local variables.

Similarly openTimer and closeTimer should not be public APIs.

Changes:
* Add an abort method for purpose of aborting existing requests.

Change-Id: Ic2add9c611990bf80e8b80ab154563f6551a77ea
2016-05-31 16:25:03 -04:00
jdlrobson 765aa40cc1 Replace use of jStorage with mw.storage
Use the standardised MediaWiki storage system for the simple use
case of disabling/enabling Popups. The jStorage library is 12kb
uncompressed and cannot be justified for this usage (and if it becomes
the standardised library mw.storage will begin using it)

This means that browsers with full localStorage or no localStorage
support will not be able to disable Popups. Given the current ecosystem
of the web - localStorage is widely supported - and the fact that grade
A browsers enjoy localStorage support - this is not a problem.
See https://github.com/wikimedia/mediawiki/blob/REL1_27/resources/src/startup.js#L59

Changes:
* Stop using jStorage
* Cleanup and migrate previous values in jStorage with plan for removing this
in a month.

Bug: T136241
Change-Id: I6dac2911e84d6cb20731be34add01576cf407c46
2016-05-30 12:30:38 -07:00
jdlrobson feb0c76381 Render settings via template
This improves readability and separates the HTML from the
JavaScript

Change-Id: Ib765d78890b9aeb05940df00160790b01751a36b
2016-05-30 10:49:17 -07:00
Baha 017cb24d4a Disable Popups when the Navigation Popups gadget is enabled
Bug: T135628
Change-Id: I788932d169d6940e8f9d5112f973b24c76fa856b
2016-05-27 09:16:54 -04:00
jenkins-bot 269ad09c27 Merge "Use exchars instead of exsentences for the extract" 2016-05-26 18:21:24 +00:00
Joakin ea072139df Use exchars instead of exsentences for the extract
To avoid sentence parsing bugs in other languages.

We have to artificially remove the always-added ellipsis from textextracts to
mimic previous behavior, and we'll add ellipsis via CSS afterwards.

Bug: T135824
Change-Id: Idf27f2fd18f7197e588c609eeb62ac8fc80626d7
2016-05-26 19:00:11 +01:00
jenkins-bot 0071ea9fc6 Merge "Send dwelledButAbandoned action for links when popups are not enabled" 2016-05-26 12:31:02 +00:00
Baha dee5f7470e Send dwelledButAbandoned action for links when popups are not enabled
This action was sent correctly when popups are enabled, but not when
disabled. This commit sends this event also when popups are disabled
so that we are able to compare the metric with popups enabled vs disabled.

Additional changes:
* Chain events in resources/ext.popups.targets/desktopTarget.js
* Add a separate property to the list of defaults and fix formatting.

Bug: T131315
Change-Id: I426f0a1a735b8fe6b16f3d2695d9099dd0d0469b
2016-05-26 11:57:42 +00:00
Baha 107d6edd37 Fix float on wikis with long settings messages
* Remove floats;
* Get rid of margin collapsing so that the space is easy to understand;
* Fix invalid rules.

Bug: T135629
Change-Id: I3bfaf137e4d02f0dc809c809edac9b28cb4bdc3a
2016-05-25 16:20:39 -04:00
Baha aba43fd560 Switch to Schema:Popups revid 15597282
Bug: T131315
Change-Id: I2ed18e00afb3e355327b417e68e5930b46d49086
2016-05-24 14:26:46 -07:00
Ricordisamoa 058b863304 Unswap langcode and langdir variables in popup.mustache
Broken by commit 27d811a173

Bug: T135758
Change-Id: Ia9b92b4c91e54f921ad220b7818af6f245e29b61
2016-05-20 07:24:18 +02:00
Sam Smith 41dc6d396a Conditionally enable Popups
Changes:
* Disable the desktop target by default
* If the user is in the experiment condition, then enable the desktop
  target
* Add PopupsExperiment config var, which, if set to true, causes: the
  experiment config to be added to the output as config vars, as well as
  the ext.popups.desktop Resource Loader module

Bug: T132604
Change-Id: Ia71ca924c3e2ec2ee0b0191ea2573fa7ff5e8a7e
2016-05-19 23:47:02 +00:00
Sam Smith c4667fd133 Convert isUserInCondition from async to sync
As there are an unmanageable amount of synchronous checks of
mw.popups.enabled, convert mw.popups.experiment.isUserInCondition to a
synchronous method.

Follow on I4959749.

Bug: T132604
Change-Id: Ide07e62868c77bfcd78af58dcec7303a35a72157
2016-05-19 14:06:17 -07:00
Sam Smith e9ddc8328d Handle user explicitly enabling/disabling feature
Follow on I4959749.

Bug: T132604
Change-Id: I4e6780f17b0423823295be9410a4343150e1e562
2016-05-19 19:10:53 +01:00
jenkins-bot 31430aefd8 Merge "Add ext.popups.experiment module" 2016-05-18 19:06:29 +00:00
Jdlrobson eff7dc95e6 Revert "Add properties that will be logged with each EL request"
Breaks EventLogging as the old schema is not compatible with these
new properties. We will reapply the patch later when in a better state.
My bad for merging.

This reverts commit ca20031a0e.

Change-Id: Ia961e0f99339f9045af9d0a4653599a48518cc95
2016-05-18 18:47:16 +00:00
Sam Smith 3f03d681c9 Add ext.popups.experiment module
Changes:
* Add the ext.popups.experiment module
* Add the mw.popups.experiment.isUserInCondition function, which
  returns a promise that resolves with true if the user is in the
  experimental condition, otherwise false
* If the experiment isn't configured, i.e. wgPopupsExperimentConfig is
  null or undefined, then the user isn't entered into the experiment,
  providing a kill switch

Bug: T132604
Change-Id: I49597494273e3862711a32e4951c8598e6c8bf59
2016-05-18 17:28:46 +01:00
jenkins-bot 85b27d8de8 Merge "Do not directly manipulate the cached object" 2016-05-18 09:28:41 +00:00
jenkins-bot 95d16e75de Merge "Add properties that will be logged with each EL request" 2016-05-17 21:42:53 +00:00
jdlrobson 69fff17141 Make SVG thumbnails show for Internet Explorer
IE9 does support SVG if you use  setAttributeNS.
Thus stop whitelisting SVG support there.

Don't worry about fallback rendering of SVG for IE < 9
- those that do not support SVG also do not
enjoy ResourceLoader JavaScript support.

Bug: T134979
Change-Id: I1d3d9c4c622e98df19a5e9dc4101b44242e07178
2016-05-17 20:27:09 +00:00
Baha ca20031a0e Add properties that will be logged with each EL request
Bug: T131315
Change-Id: I16f5f8170174200bb20c6bcc2835efd136d752ff
2016-05-16 16:39:07 -07:00
Baha cda1ffe425 Use mw.eventLog.Schema to log EventLogging events
This change is an intermediate step in our transition
to logging a variety of events easily.

Also:
  * Some events may need sendBeacon support and may not be
    logged if the navigator does not support sendBeacon.
  * Do not load schema code if EventLogging is not available.

Bug: T131315
Change-Id: Iff939577f65f1c6c71701dd6967939445385fb70
2016-05-16 17:29:48 -04:00
jenkins-bot 5f100a5de4 Merge "Rewrite createPopup with template for better readability" 2016-05-13 18:25:15 +00:00
Prateek Saxena 21b856f7f7 Use correct selector for div's that hold PNGs when SVGs aren't supported
* The classes in ext.popups.core.less apply to the thumbnail
which reside in .mwe-popups > div > a.popups-discreet
The thumbnail is thus not a direct child of the div parent of
popups-discreet.
* The margin-top is unnecessary.

Change-Id: If91140a55baa1aef36483002de681503c2690cf6
2016-05-13 18:12:11 +00:00
jdlrobson 7f3be6dcd4 Do not directly manipulate the cached object
Popups currently makes use of a render cache. Using a cache in this
way without protection causes problems (I will look at cleaning this up)

The hack below it
`mw.popups.$popup.html( mw.popups.$popup.html() );`
will lead to the html of the cached object being wiped out on IE9 meaning
future loads will show an empty popup.

Add a unit test (which previously would fail in IE9) to protect against this
in future.

Bug: T68496
Change-Id: Icef784fb389b0ab1856e2ba7464c9423ebcd62ab
2016-05-12 16:56:19 -07:00
jdlrobson 27d811a173 Rewrite createPopup with template for better readability
Change-Id: If646623ba4ebf5dfac2a94c73b6f053131739767
2016-05-12 16:19:13 -07:00
Sam Smith 26be90dc5a Add client-side validation of PopupsSurveyLink
Changes:
* Extract survey link element creation into
  mw.popups.render.renderers.article#createSurveyLink
* Make #createSurveyLink throw an error if the URL doesn't start with
  https or http
* Add unit tests that cover the new behaviour of #createSurveyLink

Bug: T129177
Change-Id: I8b61cb0e94ab4e30bc894c279bb05918ecc7719e
2016-05-11 14:53:30 +01:00
Sam Smith dab4f55e0d Annotate survey link with rel=noreferrer
Don't leak referrer information or `window.opener` to the survey hosting
site.

Bug: T129177
Change-Id: I828bd01391bc1e034fe5655d89209b83f192b112
2016-05-11 14:02:55 +01:00
jenkins-bot 9a9b4abdea Merge "Allow brackets in createImgThumbnail" 2016-05-11 10:16:07 +00:00
jdlrobson f3d23c975f Allow brackets in createImgThumbnail
Changes:
* Quote the URL with double quotes when generating the background-image
  rule

Bug: T129177
Change-Id: I74748c7efe67954272ce0a539455b0b00001a26a
2016-05-11 10:18:49 +01:00
Prateek Saxena 46f69baf38 article: Remove bracketed text from the title before looking to bold it in the extract
Bug: T69224
Change-Id: I5306101a7acd3c33c55989e97c7581a403594645
2016-05-10 21:51:57 +00:00
Prateek Saxena ed327f8ee6 Minor refactor
1. Add comment for empty SPAN when there is no thumbnail
2. Use footer instead of div.timestamp-* as it also contains
   the settings icon now.

Change-Id: Ia6fca32770596c980b33a9a6e58bb08b661b90d1
2016-05-10 14:23:08 -07:00
Baha 72dd872781 Add QUnit test for ext.popups.settings
Test pieces that make sense, everything else is already
covered in cucumber tests. See the following commit for more
info: I55f311b6b8845e6ebf4cc5698758afd1f9042a45.

Bug: T133025
Change-Id: I474c1569494601ae5865dcfba22ea728220ff8df
2016-04-28 16:13:20 -04:00
Baha 06b49dc6b8 Add QUnit tests for ext.popups.logger
Bug: T133024
Depends-On: Icb1e6ddc8f95da5e4b4de2916d292694c11ba731
Change-Id: I73fee2e3351de357f8f60bf6287a876e245117df
2016-04-20 18:33:53 +00:00
jenkins-bot 9f83c10ad8 Merge "Bold only first instance of title in extract" 2016-04-18 10:14:19 +00:00
Baha 8f3832e68a Add X-Analytics request header when fetching popup data
API requests to fetch the hovercards data now include
`X-Analytics: preview=1` header so that HoverCard hovers are
easily distinguishable in Hive.

Bug: T129425
Change-Id: I69df51a627951c4373b3b7463ab5b2c0a129faa1
2016-04-13 14:32:00 -04:00
Prateek Saxena 5f92196541 Bold only first instance of title in extract
Bug: T132523
Change-Id: I3145186264edd23ca898365ae55184cbe96ada6a
2016-04-13 21:24:42 +05:30