Page MenuHomePhabricator

VisualEditor: [Regression wmf21] In media dialog, "Size values are invalid" message does not show up, when choosing a different image and then setting its size to 0 x 0 px
Closed, ResolvedPublic

Description

size invalid message shown

Environment- Test2, beta

1> Go to media settings dialog for any image.
2>Go to Advanced Settings -> In Image size, click on custom and change its size to 0x0px.Notice message saying “Size values are invalid” shows up under the Make Full size button.
3> Now choose a different image and for that image repeat steps #2.Notice “Size values are invalid” does not show up.

Also if the image size is invalid then the insert button should get disabled, preventing the user from inserting it.


Version: unspecified
Severity: minor
OS: Windows XP

Attached:

s1.png (668×1 px, 93 KB)

Details

Reference
bz70861

Event Timeline

Created attachment 16474
size invalid message not shown when choosing a different image

Attached:

s2.png (668×1 px, 93 KB)

gerritadmin wrote:

Change 160789 had a related patch set uploaded by Mooeypoo:
Validate size widget when activating/deactivating apply button

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

gerritadmin wrote:

Change 160790 had a related patch set uploaded by Mooeypoo:
Set size to default on 0 value

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

gerritadmin wrote:

Change 160789 merged by jenkins-bot:
Validate size widget when activating/deactivating apply button

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

I can still reproduce this on Betalabs.

gerritadmin wrote:

Change 160790 merged by jenkins-bot:
Set size to default on 0 value

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

Right now, if you try to input 0 , the widget just does not let you to enter it, but as soon as you type some non numeric character, it automatically sets to default size , I think both cases should have same action: set to default size.

We didn't want to force an input on the user, which is why non-numeric input is allowed (but invalid.) The only problem with 0's is that practically speaking, an empty input and 0 are more or less the same, so 0 (or empty) assumes the user *meant* to say they don't want to set values -- which means they wanted to go with "default".

Changing the value automatically when the user inputs a non-numeric character can be annoying if it was done by mistake. If I want to type 156 and I accidentally type 15t, it wouldn't be very comfortable to have the widget automatically make my values into default.

I think that the 0-to-default makes sense, but the other behavior will be more of a hinderance to the user than help.

(In reply to Moriel Schottlender from comment #8)

We didn't want to force an input on the user, which is why non-numeric input
is allowed (but invalid.) The only problem with 0's is that practically
speaking, an empty input and 0 are more or less the same, so 0 (or empty)
assumes the user *meant* to say they don't want to set values -- which means
they wanted to go with "default".

Right now, it is not doing what you are suggesting here,the widget just does not let you type anything when you try to input 0 and "not" making it "default"

Changing the value automatically when the user inputs a non-numeric
character can be annoying if it was done by mistake. If I want to type 156
and I accidentally type 15t, it wouldn't be very comfortable to have the
widget automatically make my values into default.

But, this is what it is doing right now.It is changing the value automatically to default when user inputs non-numeric character.So, I suggested to have similar behavior for both cases.

I think that the 0-to-default makes sense, but the other behavior will be
more of a hinderance to the user than help.

(In reply to Rummana Yasmeen from comment #9)

Right now, it is not doing what you are suggesting here,the widget just does
not let you type anything when you try to input 0 and "not" making it
"default"

It's making it default, which erases the visible values. Technically it does what I explained; practically it does what you say. It's the same thing, but different user experiences.

I am not sure if it's right to change the current behavior on that, though.

But, this is what it is doing right now.It is changing the value
automatically to default when user inputs non-numeric character.So, I
suggested to have similar behavior for both cases.

I see what you're saying, but my point is that there's a difference conceptually between the two cases, so I think it's okay that the two behaviors are different two.

When a user inserts "0" value, they likely meant for the image to have no dimensions, which effectively means it is defaulted.

When a user inserts a non-numeric letter like "a" or "-" it is very likely that it's an error and even if it's not, we have no way of knowing what the user meant. Hence, we don't assume anything and instead we mark the values as "invalid".

When the image size is default, inserting a non-numeric char changes it to custom.And if i keep entering non-numerics, then it gives the message "Size values are Invalid", but the "Insert" button stays active.

It gets disabled only when you further make any changes to the settings. for ex-uncheck the “Wrap text around this item” checkbox or change the Image type to Basic /Frameless.Notice when any change is made to the settings then only the Insert button gets disabled and not when an invalid size is detected.

gerritadmin wrote:

Change 164784 had a related patch set uploaded by Mooeypoo:
Emit change event when dimensions change in MediaSizeWidget

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

gerritadmin wrote:

Change 164784 merged by jenkins-bot:
Emit change event when dimensions change in MediaSizeWidget

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

Not pulled through yet, but will be.