wikimedia/mediawiki-skins-MinervaNeue
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/skins/MinervaNeue synced 2024-11-23 22:03:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Make Talk Page Support Sections With Any Valid Id

Browse source 
Clicking on any talk page section should now open it regardless of the
characters in it. This includes ascii and non-ascii characters.

There are two changes done here:

1) When a user clicks on a section, `window.location.hash` is set to the
percent encoded version of the associated id attribute of the section.
This is important because, unfortunately, different browsers can encode
characters that do not conform to RFC 3986 (illegal URI characters) [1]
differently when calling `window.location.hash` again [2] (e.g. chrome
encodes `>` as '%3E' while safari leaves it as '>').

2) Register the encoded version with OverlayManager. OverlayManager will
simply do a strict string equality check when checking if the current
path matches. Because the browser will navigate to the percent encoded
version in step one and this version does not contain any illegal URI
characters, `window.location.hash` should give back the same percent
encoded string and the paths will match across browsers.

**Why not put this logic in OverlayManager?**

Alternatively, we could make OverlayManager decode the current route's
hash fragment and make it compare that with the unencoded version of the
id similar to the work in
I9cdaf3b01c2e5fe25512b6c18dcf6787c4422abd. However, ids with the '%'
character would then pose problems (e.g. `decodeURIComponent('100%')`
throws an error). This would require extra logic in OverlayManager to
differentiate client supplied '%' characters from browser encoded ones.

Making OverlayManager responsible for normalizing hash fragments will
make it more complicated than it already is. However, making the client
only register routes in OverlayManager that conform to RFC3986 from the
start avoids all of this logic at the expense of making the client make
one call to encodeURIComponent (if necessary).

If this patch is agreed upon, then the next step would be to change the
jsdoc `add` method  in OverlayManager to be explicit that it will only
work with URIs that conform to RFC3986 and the client should percent
encode if necessary before registering.

[1] https://tools.ietf.org/html/rfc3986
[2] https://bugs.webkit.org/show_bug.cgi?id=180396 (Thanks to TheDJ for
pointing this out)

Bug: T238364
Change-Id: Idc2cfac51c40f585c5d43713d8edf848b10424fd
This commit is contained in:
Nicholas Ray 2020-01-14 17:06:17 -07:00 committed by Jdlrobson
parent 4d67ae1044
commit f38d025b38
1 changed files with 9 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
resources/skins.minerva.scripts/talk.js
View file

@ -90,7 +90,13 @@ module.exports = function ( mobile ) {
$heading = $( this ),
$headline = $heading.find( '.mw-headline' ),
sectionIdMatch = $heading.next().attr( 'class' ).match( /mf-section-(\d+)/ ),
headlineId = $headline.attr( 'id' );
headlineId = $headline.attr( 'id' ),
// T238364: Before registering a route with OverlayManager, we need to
// encode the id first to ensure it forms a valid URI in case the id
// contains illegal URI characters as defined by RFC3986. This avoids
// inconsistencies with how different browsers encode illegal URI
// characters.
encodedHeadlineId = encodeURIComponent( headlineId );
if ( sectionIdMatch === null || sectionIdMatch.length !== 2 ) {
// If section id couldn't be parsed, there is no point in continuing
@ -100,7 +106,7 @@ module.exports = function ( mobile ) {
$heading.on( 'click.talkSectionOverlay', function ( ev ) {
ev.preventDefault();
window.location.hash = '#' + headlineId;
window.location.hash = '#' + encodedHeadlineId;
} );
@ -110,7 +116,7 @@ module.exports = function ( mobile ) {
// however cache it to data for cooperation with the 'read as wiki page' button.
$headline.data( 'id', headlineId );
overlayManager.add( headlineId, function () {
overlayManager.add( encodedHeadlineId, function () {
return createTalkSectionOverlay( $heading, $headline, sectionId );
} );
} );