Last modified: 2012-12-08 18:21:04 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42585, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40585 - Unable to change any user preferences if existing wpwatchlistdays value is a non-integer
Unable to change any user preferences if existing wpwatchlistdays value is a ...
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
User preferences (Other open bugs)
unspecified
All All
: High major (vote)
: ---
Assigned To: Tim Landscheidt
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-27 22:33 UTC by Richard Guk
Modified: 2012-12-08 18:21 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Richard Guk 2012-09-27 22:33:16 UTC
Special:Preferences validation on enwiki is preventing saving new or existing preferences where the watchlist duration is a non-integer number of days.

In [[Special:Preferences#mw-prefsection-watchlist]] the wpwatchlistdays input element has the attribute type="number", ensuring client-side validation that the number of days is numeric.

But the step="..." attribute is not set. For type="number", W3C standards treat a missing step as implying a step value of 1, so non-integers are rejected.[note 1][note 2]

But wpwatchlistdays has previously permitted any positive decimal number (up to prefs-watchlist-days-max). For example, a user could have a valid value of 0.5, which would show changes within the previous 12 hours.

In addition to preventing new non-integer values from being set, this excessive validation prevents the saving of ANY changes to user preferences if there is an existing non-integer value previously saved.

Worse still, on Chrome, when the validation fails, the mw-prefsection-watchlist tab is not activated if it is not already showing. So someone with an existing non-integer wpwatchlistdays setting who is attempting to save unrelated changes to a different prefsection will see no hint as to why their preferences are not saved. (In Chrome Developer Tools, a JavaScript console message states: "An invalid form control with name='wpwatchlistdays' is not focusable." But this is not visible to most users.)

This bug might have arisen now as a result of client-side validation being activated by the switch to HTML5.

I'm experiencing this using Chrome 22.0.1229.79 and a pre-existing prefs-watchlist-days user preference of "0.25" (i.e. 6 hours).

The simplest fix would be to add an additional attribute to the input element to override the default:

  step="any"

It might also be appropriate to specify min and max attributes, but these will be caught by server-side validation so their absence is not problematic.


[note 1] http://dev.w3.org/html5/spec/states-of-the-type-attribute.html#number-state-(type=number) "The default step is 1"

[note 2] http://dev.w3.org/html5/spec/common-input-element-attributes.html#concept-input-step-default "If the attribute is absent, then the allowed value step is the default step multiplied by the step scale factor. Otherwise, if the attribute's value is an ASCII case-insensitive match for the string "any", then there is no allowed value step."
Comment 1 Richard Guk 2012-09-28 16:26:51 UTC
This seems to have been caused by the fix to bug 23769.

The bugfix in Html::expandAttributes() causes the step attribute to be silently dropped. But the current W3C standard states that a missing step implies step="1", which is more restrictive than the step="any" correctly supplied by HTMLForm::getInputHTML() when the internal type == 'float'.

So an input element with type="number" causes validation error in Chrome when non-integer values specified, even though the input type is internally specified as a float.

* step attribute for float input set to 'any':
https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=blob;f=includes/HTMLForm.php#l1583

* step attribute dropped:
https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/core.git;a=blob;f=includes/Html.php#l467
Comment 2 Richard Guk 2012-10-10 08:04:57 UTC
See also bug 40909 (duelling tooltips with invalid email submission in HTML5).
Comment 3 Daniel Friesen 2012-10-10 08:11:52 UTC
WebKit now has a UI for form validation, Opera has a nice one, and even Firefox has a UI. So the original bug /fixing/ validation by removing validation attributes is invalid now. We should just eliminate that code that drops validation.
Comment 4 Robin Pepermans (SPQRobin) 2012-11-18 17:29:59 UTC
(In reply to comment #3)
> We should just eliminate that code that drops validation.

I submitted a change for this recently, see https://gerrit.wikimedia.org/r/#/c/23995/
Comment 5 Richard Guk 2012-11-19 05:48:20 UTC
(In reply to comment #4)
> I submitted a change for this recently, see
> https://gerrit.wikimedia.org/r/#/c/23995/

Thanks. If the above change were implemented, would non-valid input generate a UI message that is visible if the user has selected a different section of Special:Preferences?

The problem with "any" being stripped previously was not only that it blocked valid input, but that the user had no visible indication of why the submit had failed, because the client UI message (in Chrome at least) displays next to the relevant field, which is not necessarily visible with multipage forms.
Comment 6 Robin Pepermans (SPQRobin) 2012-11-26 04:01:57 UTC
(In reply to comment #5)
> Thanks. If the above change were implemented, would non-valid input generate a
> UI message that is visible if the user has selected a different section of
> Special:Preferences?
> 
> The problem with "any" being stripped previously was not only that it blocked
> valid input, but that the user had no visible indication of why the submit had
> failed, because the client UI message (in Chrome at least) displays next to the
> relevant field, which is not necessarily visible with multipage forms.

The change would just allow attributes like "any", and as far as I tested the problem with UI message displayed on a different section is still present. Maybe a separate bug should be submitted for this, and/or report upstream. In any case it's best not to merge Gerrit change #23995 for now because it would only make this bug worse.
Comment 7 Richard Guk 2012-11-26 06:57:48 UTC
(In reply to comment #6)
> The change would just allow attributes like "any", and as far as I tested the
> problem with UI message displayed on a different section is still present.
> Maybe a separate bug should be submitted for this, and/or report upstream. In
> any case it's best not to merge Gerrit change #23995 for now because it would
> only make this bug worse.

Fair enough. But as a higher priority, valid non-integer input should not be blocked, so a more modest fix is still needed.

The key issue with stripping "any" is that it is unexpectedly making the default HTML5 validation *more* restrictive than intended.

So, even if the complete validation is still stripped out to avoid multi-tab UI confusion, step='any' needs to be permitted by Html.php as a special less-restrictive-than-nothing case.
Comment 8 Tim Landscheidt 2012-11-29 12:27:21 UTC
(In reply to comment #7)
> [...]
> The key issue with stripping "any" is that it is unexpectedly making the
> default HTML5 validation *more* restrictive than intended.

> So, even if the complete validation is still stripped out to avoid multi-tab UI
> confusion, step='any' needs to be permitted by Html.php as a special
> less-restrictive-than-nothing case.

Gerrit change #35884.  Needs to be backported to 1.19 (LTS) and 1.20 as well.
Comment 9 Robin Pepermans (SPQRobin) 2012-11-29 15:56:37 UTC
I see... Gerrit change #35884 is good, as a fix until Gerrit change #23995.

(A bit off-topic:)
First I was thinking Chrome didn't accept non-integers even with step="any", but I was entering e.g. "7.5" (which didn't work) while Chrome requires "7,5" probably due to using the Dutch locale of Chrome (whereas I was using Firefox in English). I entered 7.5 in Firefox and it appeared in Chrome as 7,5... In Opera (in Dutch) it accepts 7.5, I entered 7,5 and it saved but the value just disappeared :-S
I'm not sure what should work and what should not, but anyway that is a more general (browser) problem than this bug.
Comment 10 Tim Landscheidt 2012-11-29 18:41:42 UTC
(In reply to comment #9)
> [...]
> (A bit off-topic:)
> First I was thinking Chrome didn't accept non-integers even with step="any",
> but I was entering e.g. "7.5" (which didn't work) while Chrome requires "7,5"
> probably due to using the Dutch locale of Chrome (whereas I was using Firefox
> in English). I entered 7.5 in Firefox and it appeared in Chrome as 7,5... In
> Opera (in Dutch) it accepts 7.5, I entered 7,5 and it saved but the value just
> disappeared :-S
> [...]

I don't know the specifics of preferences, but in general "7,5" (not what the browser displays, but actually submits) should be rejected server-side for "float" HTMLForm fields so if you didn't receive a (server-side) error something seems to be broken in MediaWiki.
Comment 11 Tim Landscheidt 2012-12-08 18:21:04 UTC
(In reply to comment #8)
> [...]
> Gerrit change #35884.  Needs to be backported to 1.19 (LTS) and 1.20 as well.

- Backport to 1.19: Gerrit change #37632.
- Backport to 1.20: Gerrit change #37635.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links