Catrope has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311354

Change subject: MWInternalLinkAnnotation: Fix href normalization for special 
characters
......................................................................

MWInternalLinkAnnotation: Fix href normalization for special characters

<a href="Foo%3F"> would dirty-diff to <a href="Foo?"> and also render
as such, pointing to the wrong page.

We also called decodeURIComponent() on the href twice, which can't
have been good.

Move URI decoding and underscore normalization into getTargetDataFromHref(),
and add rawTitle for callers that need it. Put rawTitle in the
origTitle attribute, so that equivalence comparisons (decode(origTitle) === 
title)
work as intended.

Bug: T145978
Change-Id: I29331a4ab0f8f7ef059c109f6813fa670a2c7390
---
M modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js
M modules/ve-mw/dm/annotations/ve.dm.MWInternalLinkAnnotation.js
M modules/ve-mw/tests/dm/ve.dm.mwExample.js
3 files changed, 30 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor 
refs/changes/54/311354/1

diff --git a/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js 
b/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js
index 14a31f5..cef9f69 100644
--- a/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js
+++ b/modules/ve-mw/ce/nodes/ve.ce.MWTransclusionNode.js
@@ -201,7 +201,7 @@
                                var targetData = 
ve.dm.MWInternalLinkAnnotation.static.getTargetDataFromHref(
                                                this.href, 
transclusionNode.getModelHtmlDocument()
                                        ),
-                                       normalisedHref = 
ve.decodeURIComponentIntoArticleTitle( targetData.title, false );
+                                       normalisedHref = 
ve.decodeURIComponentIntoArticleTitle( targetData.rawTitle, false );
                                if ( mw.Title.newFromText( normalisedHref ) ) {
                                        normalisedHref = mw.Title.newFromText( 
normalisedHref ).getPrefixedText();
                                }
diff --git a/modules/ve-mw/dm/annotations/ve.dm.MWInternalLinkAnnotation.js 
b/modules/ve-mw/dm/annotations/ve.dm.MWInternalLinkAnnotation.js
index 588cb7e..c2dfa35 100644
--- a/modules/ve-mw/dm/annotations/ve.dm.MWInternalLinkAnnotation.js
+++ b/modules/ve-mw/dm/annotations/ve.dm.MWInternalLinkAnnotation.js
@@ -42,10 +42,10 @@
                type: this.name,
                attributes: {
                        hrefPrefix: targetData.hrefPrefix,
-                       title: ve.decodeURIComponentIntoArticleTitle( 
targetData.title ),
+                       title: targetData.title,
                        normalizedTitle: this.normalizeTitle( targetData.title 
),
                        lookupTitle: this.getLookupTitle( targetData.title ),
-                       origTitle: targetData.title
+                       origTitle: targetData.rawTitle
                }
        };
 };
@@ -88,6 +88,8 @@
  * @return {Object} Information about the given href
  * @return {string} return.title
  *    The title of the internal link, else the original href if href is 
external
+ * @return {string} return.rawTitle
+ *    The title without URL decoding and underscore normalization applied
  * @return {string} return.hrefPrefix
  *    Any ./ or ../ prefixes on a relative link
  * @return {boolean} return.isInternal
@@ -125,7 +127,8 @@
        // point. So decode them, otherwise this is going to cause failures
        // elsewhere.
        return {
-               title: ve.safeDecodeURIComponent( matches[ 2 ] ),
+               title: ve.decodeURIComponentIntoArticleTitle( matches[ 2 ] ),
+               rawTitle: matches[ 2 ],
                hrefPrefix: matches[ 1 ],
                isInternal: isInternal
        };
diff --git a/modules/ve-mw/tests/dm/ve.dm.mwExample.js 
b/modules/ve-mw/tests/dm/ve.dm.mwExample.js
index c2bfb6c..f8dc2b4 100644
--- a/modules/ve-mw/tests/dm/ve.dm.mwExample.js
+++ b/modules/ve-mw/tests/dm/ve.dm.mwExample.js
@@ -1076,6 +1076,29 @@
                        { type: '/internalList' }
                ]
        },
+       'internal link with special characters': {
+               body: '<p><a rel="mw:WikiLink" 
href="./Foo%3F+%25&Bar">x</a></p>',
+               head: '<base href="http://example.com"; />',
+               data: [
+                       { type: 'paragraph' },
+                       [
+                               'x',
+                               [ {
+                                       type: 'link/mwInternal',
+                                       attributes: {
+                                               title: 'Foo?+%&Bar',
+                                               origTitle: 'Foo%3F+%25&Bar',
+                                               normalizedTitle: 'Foo?+%&Bar',
+                                               lookupTitle: 'Foo?+%&Bar',
+                                               hrefPrefix: './'
+                                       }
+                               } ]
+                       ],
+                       { type: '/paragraph' },
+                       { type: 'internalList' },
+                       { type: '/internalList' }
+               ]
+       },
        'numbered external link (empty mw:Extlink)': {
                body: '<p>Foo<a rel="mw:ExtLink" 
href="http://www.example.com";></a>Bar</p>',
                data: [

-- 
To view, visit https://gerrit.wikimedia.org/r/311354
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I29331a4ab0f8f7ef059c109f6813fa670a2c7390
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to