Page MenuHomePhabricator

VisualEditor: TitleInputWidget should validate inputs
Closed, ResolvedPublic

Description

We have isValid() functions for similar inputs, but not this one, so it's possible to try to create a redirect to "|" (which Parsoid turns into MediaWiki:Badtitle).


Version: unspecified
Severity: normal

Details

Reference
bz71249

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:53 AM
bzimport set Reference to bz71249.

Currently the redirect section of the page settings dialog and the template placeholder page use MWTitleInputWidget.

The template placeholder page can be fixed with a simple change from value === '' to !mw.Title.newFromText( value ) in MWTemplatePlaceholderPage#onTemplateInputChange. It will no longer allow you to add a template with an invalid name. No need for isValid here.

As for the redirect settings, that seems more difficult. I don't think we should just silently ignore the value if it was set to an invalid title... And I don't think we should disable the apply button either.

(In reply to Alex Monk from comment #1)

Currently the redirect section of the page settings dialog and the template
placeholder page use MWTitleInputWidget.
The template placeholder page can be fixed with a simple change from value

'' to !mw.Title.newFromText( value ) in

MWTemplatePlaceholderPage#onTemplateInputChange. It will no longer allow you
to add a template with an invalid name. No need for isValid here.

What happens when you use a parser function or a magic word?

As for the redirect settings, that seems more difficult. I don't think we
should just silently ignore the value if it was set to an invalid title...
And I don't think we should disable the apply button either.

We show "invalid title" for link searches; presumably that's evaluated server-side?

(In reply to James Forrester from comment #3)

(In reply to Alex Monk from comment #1)

Currently the redirect section of the page settings dialog and the template
placeholder page use MWTitleInputWidget.
The template placeholder page can be fixed with a simple change from value

'' to !mw.Title.newFromText( value ) in

MWTemplatePlaceholderPage#onTemplateInputChange. It will no longer allow you
to add a template with an invalid name. No need for isValid here.

What happens when you use a parser function or a magic word?

I imagine it would break. So let's not do that.

As for the redirect settings, that seems more difficult. I don't think we
should just silently ignore the value if it was set to an invalid title...
And I don't think we should disable the apply button either.

We show "invalid title" for link searches; presumably that's evaluated
server-side?

That is shown when the page does not seem to exist (based on the search results) and we get a false-y value from mw.Title.newFromText on the given title.

This is scarily close to what I filed bug 72468 about. While filing it I was thinking that an isValid() implementation should also be provided. I'll add that to that bug.

Change 169623 had a related patch set uploaded by SuchetaG:
Introducing isValid() in MWTitileInputWidget

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

Change 169623 merged by jenkins-bot:
Introducing isValid() in MWTitileInputWidget

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

test2 has the same validation that is in betalabs:

  • cannot add Talk: as a template - 'Add template' is disabled
  • cannot add a template or a category with |(a pipe), < and > chars
  • if 'Page settings' redirection has aforementioned symbols - MediaWiki:Badtitletext is displayed. Does it need some improvement?

(In reply to etonkovidova from comment #8)

test2 has the same validation that is in betalabs:

  • cannot add Talk: as a template - 'Add template' is disabled
  • cannot add a template or a category with |(a pipe), < and > chars
  • if 'Page settings' redirection has aforementioned symbols -

MediaWiki:Badtitletext is displayed. Does it need some improvement?

That sounds like it covers all our needs.