Page MenuHomePhabricator

Fix default value for field tag, for existing and new pages.
Closed, DeclinedPublic

Description

It is not possible anymore to use a default value in the field tag on existing pages. default=now is also not working. This is a possible fix for both of these problems. See discussion for details.

Event Timeline

Changes from line 1031 till 1066.

@Jongfeli - could you attach a file with just the changes/diff, rather than the entire code?

Yes you are right but in the past I tried this under windows but did not succeed but now it seems to have worked, I hope :)

Okay, thanks. I just tried out this code, and as far as I can tell it suffers from two bugs:

  1. The default values don't show up in the form, when you go to edit an existing page that will be getting default values - they show up in the saved page, but not in the form.
  1. Blank values are treated the same as missing fields.

For #2, the issue is that there's a difference between a page whose template call contains something like "ABC=" (no value), and one that doesn't contain the field "ABC" at all. In the first case, the default value should not get set in the page, while for the second case it probably should. I don't know how hard it would be to get the SF code to handle one but not the other. Maybe SF's $cur_value variable is already set to blank in one case and null in the other; I don't know. (If so, this would probably be pretty easy to fix.)

#1. I don't understand. With the patch applied I tried to replicate this but on our wiki the default values used in the form are showing up in the actual form and on the saved page. The patch seems to be working fine. It works with the edit with form tab (for existing pages) or {{#forminput:... for new and existing pages. I tried this on a blank wiki with the following extensions (installed with composer).

  • MW 1.26.3
  • PHP 7.0.7
  • SMW "dev-master"
  • SF "dev-master"
  • open-layers "dev-master"

#2. If a value for a parameter is empty (in the form) the parameter is not saved as: |exampleparameter= on the page (is this an empty string or null?). In the past SF did save an "empty" parameter on the page when there was nothing to save. For the current situation this means when in the form the default is deleted, the field is empty and the page is then saved there is no mention of that parameter on the page anymore, there is no |exampleparameter= on the page. This would mean there is only "something" or "nothing" (null).

I did change the detection on $form_field->getDefaultValue(). Using !empty is not a good idea.

Thanks for the new patch. I tried out #1 again, and couldn't reproduce it, so either this new patch fixed it or it was just a mistake on my part.

Problem #2 is still there, unfortunately. Your change seems like it's on the right track, but it's clearly not enough. Did you test this?

Yes I did but I think it is the story of the chicken or the egg. If your example page was created with SF without a value for ABC then it is not saved on the page like ABC= (no value). You can not check something that is not there. If you would manually add ABC= (no value) to the page in the template call and the field has a default value in the form then it will be shown in the form as soon as you edit the page with SF. If you then delete the default value again and save the page with no value for ABC it will be deleted from the template call. I must say that we always use parameters with an empty default on its own like: {{{parameter|}}}.

In short, the default for a field should not be shown when there is "ABC=" (no value) in the template call? This seems to be impossible because SF does not store "empty" parameters in the template call. The only way this could happen is when you come from an older SF version. Does SF behave differently when the parameters are not used to set SMW properties?

I did try to check for "ABC=" (no value) but I could not get it to work and it does not matter because when the page is saved again without a value it will get deleted from the template call anyway. It could very well be that I am completely missing the point :) Is it possible that you put an example on the SMW sandbox so I can see what you mean? Thanks.

No, I don't believe that is correct - if a field is blank, the resulting template call will still contain the field name, unless you use the "strict" parameter or something like that. Are you definitely not seeing the field in the page source if you set the value to blank?

No, the field is not in the page source when it has a blank value. you can test this in the SMW sandbox by entering a value for Defaultvalue1 saving the page and then blanking the value in the next edit.

I did find the topic on the talk SF talk page, see here.

Ah! Yes, that's true. Sorry - I guess I was thinking of SF's old behavior.

Well, this presumably has to change if this new feature is going to get added, no? I mean, forms have to go back to adding all the fields to the template call, even the blank ones. Otherwise, the code will have no way to know whether a certain form field is new or not. Would you agree with that?

No, I do not think it is wise to change this empty field behavior back to he way it was years ago. The default values in the field tag used to work just fine on new and existing pages, it is not a new feature I am merely trying to fix it :)

I agree with you that we should check if a field is empty or not, to be backwards compatible with older wikis that used older versions of SF. In the new patch I added some extra code to check if the field is there at all and if so check if it is blank or not. The default will not be shown in this case but when the page is saved the field will be deleted from the pages when it is empty and this is correct behavior.

Now it is working exactly like it did in SF 2.4.1. I tested this with our new (test wiki) and our production wiki next to each other.

In our use case the whole point of using default values is mainly in mandatory fields. In some cases we already know what the value should be so the users do not need to set it manually. The new patch below:

Perhaps, then, this behavior should only be for fields that have a default value and are mandatory?

No I don't think so. Fact is that empty fields are not saved on the page anymore since SF 2.7. To me this seems to be the correct behavior.

Sorry, I think you misunderstood my question. I meant: perhaps this feature you're adding, of default values replacing blank values when editing an existing page, should only be done when the field is mandatory?

Ok, well initially the blank value will not be replaced by the default when the page is edited. Only when the page is edited again the default will be used because, by that time, the field is not on the page anymore. Any empty field that is stored on the page will disappear anyway when pages are edited with SF 2.7 and higher.

Empty fields saved on a page that where/are mandatory should not even be possible?

Yes, it's possible, because these mandatory fields can be newly-added - added after the creation of the relevant wiki pages. Isn't that the whole point of this feature - to handle fields that have been recently added to the form and template?

Yes that is possible but the feature is to be able to alter the parameters in the field tag including the default in a form and that these changes apply to existing and new pages. This feature has always been part of SF before 2.7, you where always able to alter forms anyway you wanted and this also applied to the defaults.

We do need to take into account that that the field is on the page is there but is empty. When, at the same time the field is made mandatory then the default will not be shown and someone will have to manually interact. In this case the user is aware of the fact that the field is now mandatory and it is not filled automatically with the default. For new pages it will work with the default because that is what we want. As far as I can see the behavior is exactly the same as in SF 2.4.1

Hi - there's really no point in bringing up what SF's functionality used to be in previous versions. It doesn't matter how SF used to behave; what matters is how SF *should* behave. And that's what we're trying to figure out now.

I have no idea what you meant by "When, at the same time the field is made mandatory then the default will not be shown".

Anyway, I'd like to hear why you think a default value should always overwrite a blank value, even when the field is not mandatory. What if the user wanted the value to be blank?

Ok , you are right, the past is the past :)

First, with the last patch the default value will NOT overwrite a blank value. It stays "blank" and it does not matter if the field is mandatory or not. But you can not have a mandatory field that stays blank.

Second, SF 2.7 and later will not store blank fields on a page that are maintained with a form. If a user wanted the field to be or stay blank then it will simply disappear form the page.

Okay, sorry, I think you misunderstood my question again. When I wrote "blank value", I really meant "blank or missing value", since, with SF's current handling (from 2.7 on, as you note), the two are basically equivalent.

Let me put it this way:

A form, "Employee", contains a field, "Department", which is not mandatory but contains a default value of "Marketing". It appears in the form as a dropdown.

User "Bob" fills out the form to create a page about himself, and decides to set the "Department" dropdown to blank, because he doesn't think any of the given choices describe him. He saves the page, and the resulting page contains no "Department" field.

Then, user "Charlie" edits the "Bob" page with a form a month later, in order to fix a typo. It looks like, with your current patch, the value for "Department" will then change from blank/missing to "Marketing" in the page. Is that true? And if so, do you think that's the right behavior?

I understand but in my opinion it is the right behavior because that is where the default value is for. However you are right, in your example Bob will be working in the "wrong" department after Charlie edited his page :) Which is probably incorrect. An SMW wiki is as good as the people who build it:

  1. Bob was able to fill in a blank value because he did not find the correct department.
  2. The wiki maintainer used the "wrong" default. It should be something like "Unknown".

If you want to prevent these things from happening SF should store blank values on the page. Maybe it is possible to do this in the field tag like {{{field|ABC|default=test|saveblank}}}.

Hello Yaron, are there any plans to get the default value fixed for all pages, new and existing like discussed here?

Jongfeli - I have no specific plans; I still have the objections I did before, and I don't think a "saveblank" parameter is a good idea.

Maybe this can all just be handled in the template? After all, the reason you're asking for this feature seems to be only about when a new field gets added to a template, and you want that field (and its default value) to get added to every page that calls that template. So let's say, again, that the field is called "Department" and its default value is "Marketing". What about just changing tags like "{{{Department|}}}" in the template to "{{{Department|Marketing}}}"? Wouldn't that have the same effect?

Well you are making it very hard for me here :) The saveblank option was merely a suggestion.

As far as I can see the default= tag for existing pages is lost since SF 2.4.1 . I don't know if this was by accident or "on purpose" but the documentation does not and did not mention anything about the default tag being only applicable to new pages, it has always worked on existing and new pages in SF before 2.4.1 this means powerful functionality is lost, being able to set new and old values without user intervention on existing pages.

Providing the default value from the template does not work, it is not passed to the form.

The problem in earlier versions was, that if a field had a default value and the user deliberately blanked it, on next edit the default would show up in the form again. If an unsuspecting user did not notice this, values would start to creep in again that were unwanted. Back then, I very welcomed this fix. Unfortunately, we have the known problems now instead.

Seeing that this kind of issue creeps up every now and then on phabricator, on the discussion page, and in the mailing list tells me, it's a quite controversy topic. Working with dynamic forms that are compiled according to user input (and I don't mean show_on_select), my life would be way easier, if we had a solution for this. Besides this very exotic setting, I also think it is not far fetched that object classes and therefore their property/cargo tables change over time (one wants to adapt, extend, develop). And with this change, the corresponding form needs adaption, too. As of now, an object - once created - could cause difficulties when trying to converted it a new schema. Or at least forcing manual work. And seeing some issus on the mailing list where wikis deal with thousands of objects that is some kind of work.

My work with SMWikis would not be possible and the acceptance in my department for such wikis would not be there without Semantic Forms. Therefore, I am immensly grateful, that this powerful extension exists and that Yaron and others spend so much time maintaining and developing it. But I have to take this problem of "static object classes" into consideration every time I do object / relation design. And I don't know what to do when this issue becomes a no-go. :(

Enough with the sermon. I humbly ask again to take into consideration addressing problems like:

  • if a field has a default value but is blanked by the user, I would expect sf to ignore the default on consecutive edits
  • if a formerly hidden field (meaning, a field being target of a "show on select") is presented to the user, I would expect sf to use the default value
  • if the form definition has changed and a field was added that has a default value, I would expect sf to use the default value, as well

I know, it's a ton of work. I would help if I could.

Regards.

BTW: For more see also T38505 and T128340.

My work with SMWikis would not be possible and the acceptance in my department for such wikis would not be there without Semantic Forms. Therefore, I am immensly grateful, that this powerful extension exists and that Yaron and others spend so much time maintaining and developing it.

I totally agree.

But we are almost there, the only thing not working with the patch is:

if a field has a default value but is blanked by the user, I would expect sf to ignore the default on consecutive edits

With the patch provided this actualy works the first time a page is edited with blank fields, SF will not show the default value but when the page is saved the "empty" field is deleted from the template call because as of SF 2.7 empty fields are not stored on the page anymore in the template call.

If saving empty fields would be made possible again via a saveblank option on field or form level or for the complete wiki...

Hi guys,

Various thoughts:

  • Thanks for the nice words about SF. And I, too, want this issue to be resolved in one way or another.
  • Jongfeli: I really don't think it's true that default values were added when editing existing pages, in all SF versions before 2.4.1. In any case, as I've said before, it doesn't matter what SF used to do. (I thought you agreed with me on that, but I guess you changed your mind.)
  • Oetterer: that's a helpful breakdown of the cases that need to be addressed by default-value handling. To those three I would a fourth: newly-added instances of multiple-instance templates. Though I think for that case the default-value handling already works correctly, i.e. the default value shows up in the form.
  • It seems to me that by far the big issue here is the third case Oetterer listed: when a field with a default value is added to the form after pages have already been created with that form.
  • I don't think a "saveblank" parameter would be useful, because a form creator has no way to know, when they first create a form, whether any particular field should get it or not. What would work instead is to simply apply that behavior - that fields with no value should get placed in the page anyway - to all fields. Whether that's a good idea is another story.
  • Jongfeli - you're right that my suggestion of replacing, say, "{{{Department|}}}" with "{{{Department|Marketing}}}" wouldn't work - unless it was a mandatory field, but that's a special case.

I think the real solution to this does not involve default-handling at all, though. Let me explain: let's assume that the 3rd case - new fields for old forms - is really the main issue here. Now, let's take an example case; you have a form, "Employee" (I'll stick with my earlier example) that already has 100 pages created with it. Someone adds another field to the form and template, "Department", with a default value of "Marketing" in the form. (Almost everyone at the company works in Marketing.)

At this point, you have 100 pages that have incomplete information. The feature that we're discussing - have the default value always take effect for new fields - seems like it would lead to a strategy of waiting until pages get edited for other reasons, at which point the "Marketing" value will get added in automatically. (Unless the editor sets it to some other value in the form, of course.)

But what if 50 of those pages will never get edited again? That means that they'll always have incomplete information - unless an effort is made to go into all of the employee pages, and add a "Department" field for each one. Which I think is what would have to be done. In theory, this could be done either manually, or in some kind of automated way. The right approach probably depends on how many pages there are, and how likely the default value is to be the correct one.

Let's say you're in an extreme case: lots and lots of pages, and the default value is almost always the correct one. There are various ways to edit a lot of pages at once - there probably should be more - but one helpful way that already exists is the Replace Text extension. With that extension, you could do something like replace "{{Employee" with "{{Employee\n|Department=Marketing" across all pages, which would instantly add the new default value to all the relevant pages.

What do you think?

Hi Yaron

thanks for taking the time to compile this long response. :)

First, your 4th case was discussed in T126770 and should be already solved (as far as I can see).

Some thoughts about your suggested workaround (I take the liberty of calling it this). There are various ways of handling this case. Some come to mind:

  • Extension:Replace Text
  • default handling for mandatory fields
  • switching to optional fields
  • complete user documentation for all fields
  • using a bot
  • ... and probably others

What ails me is that this "ignore default values on edit" behaviour is an unexpected one. Almost all mentions of this problem start with "I found out that...", "I had a problem and it looks like SF does this ...", "During updating our wiki we stumbled on an issue with Semantic Forms", or something like that. This tells me that admins where expecting SF to behave differently. I personally, too, find it unintuitive, that defaults do not work on edit and I have the impression I am not alone. I admit that this "problem" does not occur very often, but I think often enough.

For now, I took the liberty to add a note of this on [0]. This is not meant to expose this problem, merely to have some transparency. Feel free to edit or remove it again if you don't agree.

In the long run, I am still convinced that SF would be better of differenciating between a deliberately blanked and a missing field. This would solve both points 2 and 3 of my list (and yes, this could probably only be solved by saving all (also blanked) fields).

[0]: https://www.mediawiki.org/wiki/Extension:Semantic_Forms/Defining_forms

Well, there are clearly two separate issues here. The first is what SF's ideal behavior should be when editing existing pages, the second is how newly-added fields in forms/templates should best be handled.

For the 2nd one, I'm still curious to hear what you guys think about my "mass field addition" idea. Oetterer - you wrote, "Some thoughts about your suggested workaround", but then you didn't offer any thoughts, as far as I can tell.

For the 1st one - I don't think the "unexpected behavior" argument is that strong. My sense is that either approach will lead to unexpected behavior, and I wouldn't be surprised if the other approach - of always handling default values - will be unexpected for many more users. (e.g., you go to fix a typo in a page and you find that you've now set three other fields on that page.)

I think you're right that saving all fields (which is how SF used to do it) is the only complete solution. Though, again, it's a solution for the 1st issue - I'm not convinced at all that it's a real solution for the 2nd one.

For the 2nd one, I'm still curious to hear what you guys think about my "mass field addition" idea. Oetterer - you wrote, "Some thoughts about your suggested workaround", but then you didn't offer any thoughts, as far as I can tell.

Hmm. I should have used another opener :/. What I meant was: here are some other ideas to solve #2. The handling of this problem imho has to be situational. But while I'm at it, let me elaborate. I'm gonna hop on to your {{Employee}}-example.

adding department

  • Given a departmenttree, I would not just assume Markteting for everyone (since that would produce wrong data), I would assign the tree root as a neutral element and set it with Replace Text as you described.
  • Finding out, that our administration holds data that allows me to extrapolate the correct Employee -> Department relation, I would implore a bot to enter correct data (api action sfautoedit!)

adding phone

  • Assumption: no remote or otherwise data is available. But I know the department (from the previous change). So generate a lookup template, that gives the number of the secretariat for every department and fill the result in as a fallback, if phone is not provided. Users then can enter their own via form later.

adding birthday

  • having again no remote or otherwise data available, I would have the template issue a big warning about the missing date and also sort the page in a gardening "Category:Employees with missing birthday". In this example, the date has to be provided manually. So mail every employee and watch the Gardening Category.

Long story short: Yes, there are unsually multiple ways to get around the issue. Most of them aren't trivial. But no one said that changing your data structure is trivial.

For the 1st one - I don't think the "unexpected behavior" argument is that strong. My sense is that either approach will lead to unexpected behavior, and I wouldn't be surprised if the other approach - of always handling default values - will be unexpected for many more users. (e.g., you go to fix a typo in a page and you find that you've now set three other fields on that page.)

SF is yours. And if you should decide, that SF's ideal behavior for editing pages is to ignore default values then so be it. No hard feelings. :) Would I be in be your place, I would try to clear out the assumptions you and I made (about what behavior is expected). At the upcomming SMWCon in Frankfurt you have your target audience. Ask them. Or make a mailing via smw-user list. I promise to behave :)

Jongfeli: I really don't think it's true that default values were added when editing existing pages, in all SF versions before 2.4.1. In any case, as I've said before, it doesn't matter what SF used to do. (I thought you agreed with me on that, but I guess you changed your mind.)

Yaron, my apologies for the late responds, you are right probably not in all SF versions but if I remember correctly, we have been using SF since SF 1.8 and it worked back then. No I did not change my mind, we should indeed discus how SF should behave but this does not mean that it does not matter what SF used to do in the past. Like Oetterer also said, some wiki's depend on certain functionality. In our case SF is crucial for our wiki to function properly. Again like Oetter mentioned before, also our SMW wiki would not be possible or accepted without Semantic Forms. This is meant as a big compliment because SF "can" basically make a SMW wiki behave as a multi million dollar CRM solution. This does not do justice to all the other Extensions we use and SMW itself but SF is the UI or HMI layer for a Semantic Wiki, it makes maintaining data on a wiki possible for the mere mortals among us. It is my humble opinion that SF should be a core part of SMW itself.

As for the default value behavior, Oetterer wrote:

Given a departmenttree, I would not just assume Markteting for everyone (since that would produce wrong data), I would assign the tree root as a neutral element and set it with Replace Text as you described.

It is up to the wiki developer to make the "correct" choice in a particular use case. Like Oetter, to me it makes no sense to assign a person to Department=Marketing if we don't know if that is true or not, you are assuming something and that is, in my opinion, not correct and can indeed produce data that is not true. In your example we would use something like Department=Unknown. In my opinion having field values telling you that we don't know gives you more information then no value at all. As pages get edited these values get added silently even do they are not complete. This means that the wiki developer decides that Department=Unknown is better then Department= or nothing at all but that is not possible when default= does not work on existing pages. And especially when they are made mandatory, you can not set them automatically and user intervention is needed.

We also have cases where fields are filled dynamical, in our case with data from an external source when existing pages are edited (with the ED extension, External Data) or a SMW query. Once the data is on the Wiki page it is maintained via the wiki page and the External Data source is updated with the data from the Wiki with the Data Transfer extension (XML dump of a certain category). This is very powerful but heavily relies on being able to set default values on existing and new pages. As you can see we also heavily rely on "you". You, and others obviously had the right ideas at the right time :)

As it is SF does not store empty fields on the page. This means that regardless of being able to set the default value on existing pages, these empty fields will disappear from the template call when pages get saved. It is simply not possible anymore to store empty fields. If a wiki developer decides that a default value is going to be used on a existing field that can be empty (is stored with a pre SF 2.7 version) then the actual value of the field (if it is true or not) has nothing to do with the functionality, that is the responsibility of the developer of the wiki and its users. In your example, If a user is fixing a typo and the result is Department=Marketing, Department=Unknown or Department=Replace Text so be it.

Hi guys,

Well, you made a lot of points - I don't know how much of it I should respond to. But let me bring up what I see as one glaring issue. It seems like you both agree that simply adding something like "Department=Marketing" to all employee pages is a bad idea, even if, say, 90% of employees are in Marketing - it's better to have incomplete data than incorrect data. That's a very reasonable opinion to have, and I'm sure everyone would agree with that - if not for 90% then for some percentage cutoff.

But as far as I can see, what you are talking about - putting in a default value for a new field for existing pages - would have this same effect, of adding incorrect data. As I noted before, if a user goes to fix a typo and a new default value is there in the form, chances are very good that the user won't notice the new value either while filling out the form or after saving the page. So the effect will be to have a slow-motion version of the exact same process: pages getting a "Department=Marketing" value added to them, regardless of whether or not this is correct data.

That's the big difference between creating a new page and editing an existing one: when people create a new page, they (usually) look at each field, and consider what its value should be for the current page. When people edit an existing page, I think they usually don't, and just focus on whatever they wanted to change.

Jongfeli - you brought up a "Department=Unknown" value as something that could be handled well as a default. But that's really not saying much, because "Unknown" is really just a synonym for blank. The real test is whether this makes sense for actual default values.

So what's the answer - why is it fine for users to mistakenly add incorrect data one page at a time?

Hey Yaron

But as far as I can see, what you are talking about - putting in a default value for a new field for existing pages - would have this same effect, of adding incorrect data. As I noted before, if a user goes to fix a typo and a new default value is there in the form, chances are very good that the user won't notice the new value either while filling out the form or after saving the page. So the effect will be to have a slow-motion version of the exact same process: pages getting a "Department=Marketing" value added to them, regardless of whether or not this is correct data.

And here it is. The convincing argument. At least for me. Nothing to add.

@Oetterer - wow, that's great! It's very rare for me to have a discussion where I end up convincing someone. :) I should have started with that argument, although I didn't think of it until recently.

Hello Yaron, a comment:

Jongfeli - you brought up a "Department=Unknown" value as something that could be handled well as a default. But that's really not saying much, because "Unknown" is really just a synonym for blank.

I do not want to dive unto a discussion if Unknown is a valid value or not but it is not a synonym for a blank value. Remember that it is not possible to store blank values on a page. When a parameter has no value assigned to it in the form the parameter is not mentioned on the page anymore. This is not the same as a value being blank.

Our wiki is a company wiki, things change all the time. We do however discus all these changes and how they can be implemented and then work with the result. Being able to set default values helps with this.

I do understand your concerns and your point is valid but most wiki's using SF work exactly as you describe, this could mean that not many wiki's actually use the default value option or not many wiki's run the latest SF version (yet) :)

Like Oetterer said:

At the upcomming SMWCon in Frankfurt you have your target audience. Ask them. Or make a mailing via smw-user list.

@Jongfeli - I know that blank values and "Unknown" are not handled the same way, but in terms of their meaning I would still say they are basically synonyms.

When you wrote "most wiki's using SF work exactly as you describe", do you mean that most wikis use default values when editing existing pages? If so, I don't think that's true - SF 2.7 came out over two years ago. I think most wikis are on version 3.5 or higher.

Plus, I think for most people this behavior doesn't really matter much one way or the other; I don't remember getting that many complaints/questions about it, with either approach.

Hello @Yaron_Koren,

When you wrote "most wiki's using SF work exactly as you describe", do you mean that most wikis use default values when editing existing pages?

No I mean that in pre SF 2.4.1 when new fields where put in an existing form that uses a default value they would be added just like you described.

Like you, I do not think that a lot of people use the default value like we do :)

Change 533120 had a related patch set uploaded (by markahershberger; owner: markahershberger):
[mediawiki/extensions/PageForms@master] Fix handling of defaults for radiobuttons

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

I understand the concern regarding new defaults on old pages, but it is currently possible to create restricted fields that are required and have a default.

I've provided a fix for radiobuttons -- we're using restricted radiobuttons in PF as part of a workflow -- and am going to work on other fields as well.

I know it has been a while (almost 3 years!) since this was discussed, but we recently upgraded and ran into this problem since the behavior of PF changed.

The patch was submitted prematurely. I would like to refactor PF so that it is easier to find and fix bugs.

That said, now that I have a better understanding of PF and the issue that my users are experiencing, I'll submit a new bug.

Change 533120 abandoned by markahershberger:
Fix handling of defaults for radiobuttons

Reason:
will fix and re-submit

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

Change 552091 had a related patch set uploaded (by markahershberger; owner: markahershberger):
[mediawiki/extensions/PageForms@master] Fix handling of defaults for radiobuttons

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

This import includes a demonstration. There are three pages in this demo: Test, Form:Test and Template:Test.

Visit the page Special:FormEdit/Test/Test and you'll see that when you click "show" you will see a list of radio buttons including "None" with "None" selected.

If you visit Special:FormEdit/Test/Test2 you will see that when you click "show", you will see that "B" is selected.

The default in the form is, of course "B". "None" should never be an option.

Alright, thanks. I haven't run the import, but I think I see the issue. The problem comes about when:

  • There's a radiobutton input that's both mandatory and restricted (though the same, I'm sure, holds true for dropdowns as well)
  • The user lacks permission to edit that input
  • The input either lacks a default value, or (as in this case) the user is editing an existing page that has no value set for that field.

This is a tricky situation, and I have the vague feeling that I've discussed this exact case with someone before, though I can't remember with whom. Anyway, this is a controversial opinion, but I believe that Page Forms is actually doing the right thing here: if the user lacks permission to modify that field, and it needs to be modified, then they should not be allowed to save the form.

Ultimately, I think this issue comes down to the question of whether default values should apply to existing pages, which is something we've discussed before - and which of course is the topic of this whole Phabricator task. My view, as before, is that a form should not "surprise" - it should not make changes to the page that the user/editor is not aware of. Perhaps that goes double when the user lacks permission to modify that input - where the user neither realizes, nor has any say over, the fact that they're adding this new value to the page.

Anyway, if you disagree, I'd like to hear your opinion.

  • The user lacks permission to edit that input

This is not the case. There is no restriction in the import.

Oh, yes, that's true - there's no "restricted" parameter in the field tag. I got confused because the commit message for the patch talks about restricted fields.

In any case, I still think default values should not kick in when editing an existing page - which seems to still be the heart of the issue, if I understand it correctly.

In any case, I still think default values should not kick in when editing an existing page

Even if we posit that you are correct, there is still a difference between the form when it is appears and, later, when it is edited, but without a saved value where the default value is. "None" appears after the form has been saved without a value.

It would help if you looked at the import I've given, but here is the content of the form:

{{{for template|test}}}
{{{field|switch
   |input type=radiobutton
   |values=hide,show
   |default=hide
   |mandatory
   |show on select=show=>show;hide=>hide
}}}

<div id=hide>
Nothing!
</div>

<div id=show>
{{{field|switch save
   |input type=radiobutton
   |default=B
   |mandatory
   |values=A,B,C
}}}
</div>

The switch save field is mandatory, but hidden. When the user cannot see it, they can't edit it, so "mandatory" is not enforced. This is similar to the restricted mandatory case: if the user cannot see or edit a value, it should not keep them from saving the field. So far, so good.

After saving the form, the user goes back to edit the form. This time, they show the hidden field. Since it doesn't have a saved value, the forms creator would expect that the default value is shown. Instead, "None" is shown and selected.

If they try to save it at this point, it will not save, because the mandatory field is shown, but a valid option is not selected.

Since the form is mandatory and has a default value provided, that value should be selected when the field is first shown.

The case of hidden or restricted fields without a value *should* behave similarly. Defaults for either type of field should be used when the fields are active. When the fields are not active--either because they are hidden, or the user cannot alter them--they shouldn't block and values for the fields, not even the defaults, should not be saved.

But, when the fields are active, even if the form was previously saved without a value in the field, mandatory fields should default to the given default.

I'm confused about the first thing you wrote - "Even if we posit that you are correct". Correct that the default value should not be used when editing existing pages? Because that seems to be the entire underlying issue.

Are you saying that the behavior should be different depending on whether or not the page being edited contains that template parameter, but blank (like "|switch save=") or does not contain it at all? If so, that's a reasonable argument, but I'm not sure that's what you're saying.

I'm confused about the first thing you wrote - "Even if we posit that you are correct". Correct that the default value should not be used when editing existing pages? Because that seems to be the entire underlying issue.

No.

I'm saying that None should not appear on mandatory fields. It doesn't appear initially, and it isn't valid later, so it shouldn't be available later. But it is shown later, even though it is not a valid selection.

If you think showing the form creator's default value is the wrong thing to do later (i.e. if we posit you are correct), then no option should be selected and None should not be provided as an option.

Are you saying that the behavior should be different depending on whether or not the page being edited contains that template parameter, but blank (like "|switch save=") or does not contain it at all?

I'm saying the behavior should be the same. That is:

  • None should never be displayed whenever a field is marked "mandatory"
  • PageForms should never save a page field with an empty value (e.g. "|switch save=") when a field is marked "mandatory"
  • It makes sense to display the default value selected when no value is saved--that is why the form designer specified that a hidden mandatory or restricted field has a default, after all--but if you don't think that is the right thing to do then the available choices should be displayed without any selection made and "None" not displayed.

Okay, I think I get it - it sounds like what you're talking about now is just leaving the radiobutton unselected, as opposed to having a "None" value. Which is sort of an aesthetic issue, unrelated to whether there's a default value, although it's still potentially an improvement.

Having the radiobutton input unselected is a logical approach, and I think that was what I originally planned to do back when I was first setting it up. But if I remember correctly, I decided to go with the "None" thing instead because (a) I was worried that not all browsers would support an unselected radiobutton, and some would just automatically select the first option, (b) it might not be clear enough to users that having no value selected was intentional rather than a technical error, and (c) it mirrors what's done for the dropdown input, where a blank value is shown even if it's not allowed as an option. (Admittedly, that comes off less awkward for dropdowns than the "None" thing does for radiobuttons.)

Anyway, those were/are my thoughts. What do you think?

I can see where you're coming from, but, instead of defending against a mysterious, unknown browser that might act strangely, just display a list of buttons that the user has to select and then, if a problem is reported, fix it. We do need to announce this change, though: "None will no longer be displayed when it is not a valid choice." I wouldn't say it breaks backward compatibility, but people need to know.

I don't understand what you mean by "if a problem is reported, fix it". What would the fix be?

I was talking about this:

I decided to go with the "None" thing instead because (a) I was worried that not all browsers would support an unselected radiobutton, and some would just automatically select the first option

So the "problem" would be that a browser appeared that would behave this way. Note, though, that the standard says:

If none of the radio buttons in a radio button group are checked when they are inserted into the document, then they will all be initially unchecked in the interface, until such time as one of them is checked (either by the user or by script).

(From the note at the bottom of the radio button section. Emphasis mine.)

So, a browser that behaves in the way you are concerned about would not be standards compliant. We spent the first 15-20 years of the web wrestling with non-compliant browsers and I think the browser makers have all seen the light now. If there is an exception, it is not something to be proactively worried about in this way.

This also addresses the second point. The standard says how radio buttons are supposed to act, and users are acclimated to this behavior. They're aware that radio button groups do not always come with a selection already made.

Oh, that's interesting - I never thought to check the W3C standard.

Yaron, in response to the latest patchset, you inadvertently +2'd. When you re-applied your -1, you also added:

I think my previous comments still apply to this patch...

I want to make sure I'm addressing the right thing. In one comment, you said:

It seems strange to turn just one form input class into a "true class", while keeping the others as they were - that might cause more confusion than it's worth. It's also strange to do this within the context of a bug fix. What about just fixing the bug in this patch?

I responded with my reasoning, which can be summarized as "I had to refactor the code to understand where the bug was. Once I was doing that, it made sense to use a 'real' class."

I might add that refactoring the code into smaller chunks that are more focused allow easier debugging and better unit testing.

I would also suggest that I'm willing to provide similar refactoring and tests in to the other bits of the code (after writing acceptance tests, natch) so that Radio-button isn't the only bit of code that has been refactored. I've had a similar "defaults" bug reported against the datepicker, for example.

Your other coment on gerrit was:

I can't reproduce the original bug, by the way. I added the following field tag to a form definition:

{{{field|The radiobutton|input type=radiobutton|values=A,B,C|restricted|mandatory|default=B}}}

...and it worked fine for a non-admin user account. The radiobutton was set to "B", could not be changed, and showed up as "B" in the resulting page.

Are you seeing this problem for new pages, existing pages, or both?

which I think was addressed in the previous discussion here when I pointed out I was seeing an issue with mandatory, but not restricted fields that were not yet displayed.

Yes, I'm talking about both of those comments. For the refactoring - it may be good, but, as I said, (a) it's outside the scope of this bug fix, and (b) I'm not sure that this kind of partial refactor is better than no refactor at all.

For the second comment - I think I get it. I'm curious to see what your fix is (it's hard, of course, to tell from this patch what the change was).

Removing task assignee due to inactivity, as this open task has been assigned to the same person for more than two years (see the emails sent to the task assignee on Oct27 and Nov23). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome.
(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)

Marking this as "Declined" - I remain convinced that default values should only be used when creating a new page, not editing an existing page (with the exception of new template instances within existing pages).