Commit graph

42 commits

Author SHA1 Message Date
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 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 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
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
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
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
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
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
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
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 79daacb235 Coalesce and cleanup use of then blocks
Bug: T190141
Change-Id: I345befc24397e27c965af0a593821333f2a71f21
2018-05-30 17:32:08 +01: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 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
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 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 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
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 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
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
Albert221 8feedf8368 Fix $.fn.hover is deprecated
Bug: T180861
Change-Id: I6a2cfbd26535c3e778a811e2f8ed5939123d6571
2017-12-06 00:52:19 +01:00
Ed Sanders 7f4dd1159b build: Update eslint and other linters
Change-Id: I0a20ad5d16d46afbc2a328f8e254295972f58ab0
2017-10-09 15:56:15 +01: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
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
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
Renamed from src/renderer.js (Browse further)