jenkins-bot has submitted this change and it was merged.

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


dm.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.MWInternalLinkAnnotation.test.js
M modules/ve-mw/tests/dm/ve.dm.mwExample.js
4 files changed, 31 insertions(+), 5 deletions(-)

Approvals:
  Jforrester: Looks good to me, approved
  jenkins-bot: Verified



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.MWInternalLinkAnnotation.test.js 
b/modules/ve-mw/tests/dm/ve.dm.MWInternalLinkAnnotation.test.js
index 6f8555f..b20d6ba 100644
--- a/modules/ve-mw/tests/dm/ve.dm.MWInternalLinkAnnotation.test.js
+++ b/modules/ve-mw/tests/dm/ve.dm.MWInternalLinkAnnotation.test.js
@@ -39,7 +39,7 @@
                                                hrefPrefix: '',
                                                lookupTitle: 'Foo?',
                                                normalizedTitle: 'Foo?',
-                                               origTitle: 'Foo?',
+                                               origTitle: 'Foo%3F',
                                                title: 'Foo?'
                                        }
                                }
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: merged
Gerrit-Change-Id: I29331a4ab0f8f7ef059c109f6813fa670a2c7390
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Alex Monk <a...@wikimedia.org>
Gerrit-Reviewer: C. Scott Ananian <canan...@wikimedia.org>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: DLynch <dly...@wikimedia.org>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: Jforrester <jforres...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

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

Reply via email to