Can't change RTL post direction back to LTR

[thebaer]

Originally reported on discuss.write.as:

Once a post's direction is set to "right-to-left", it can not be changed back to "left-to-right".

• What happened
  I was playing around with post direction, toggling right to left option worked, but could not revert it. Page keeps showing "Changes saved" but nothing really happens, the check next to "right to left" just does not go away, and the post direction never changes back.

• Any steps you took before the bug appeared, if it’s not something obvious
  No. Tried it a couple of times, always the same behaviour.

1. Write a post
2. Publish it
3. Click Edit next to logo
4. Click on "Edit post metadata"
5. Toggle direction option. This should work
6. Try to toggle it again, never goes away.

[cenrak]

You could resolve this in either ways:

• Server side: if rtl is set within POST request, change rtl in your page table to true. Otherwise, if rtl is not set within the request, just set its value to false. Probably you'd do this in database.go

• Client side: add this just before rtl checkbox input tag <input type="hidden" name="rtl" value=0 /> . It's a hidden input with the same name as of the rtl checkbox, with the value of 0 (mimicking unchecked). It will be sent with the POST if rtl is not set.

A couple of useful resources:

• https://stackoverflow.com/questions/1809494/post-unchecked-html-checkboxes
• https://dzone.com/articles/always-get-checkbox-post-value

[joyeusenoelle]

Well, now I'm even more interested, and I must be reading something wrong.

Going through posts.go, I find this chunk at 524, 529-545:

rtlValue := r.FormValue("rtl")
...
var isRTL, rtlValid bool
if rtlValue == "auto" && langValue != "" {
    isRTL = i18n.LangIsRTL(langValue)
    rtlValid = true
} else {
    isRTL = rtlValue == "true"
    rtlValid = rtlValue != "" && langValue != ""
}

// Create a new post
p = &SubmittedPost{
    Title:    &title,
    Content:  &post,
    Font:     appearance,
    IsRTL:    converter.NullJSONBool{sql.NullBool{Bool: isRTL, Valid: rtlValid}},
    Language: converter.NullJSONString{sql.NullString{String: langValue, Valid: langValue != ""}},
}

And in templates/edit-meta.tmpl, at line 258:

<dd><input type="checkbox" id="rtl" name="rtl" {{if .Post.IsRTL.Bool}}checked="checked"{{end}} /><label for="rtl"> right-to-left</label></dd>

The value attribute of checkbox#rtl isn't set in the template file, which means that in POST, we'll either get rtl=on or nothing.

("on" is the default value for an unvalued, checked checkbox in HTML. You can test this in the web console by opening the "Edit Metadata" page and logging document.GetElementById('rtl').value to the console. In the case of the value not appearing, FormValue will return an empty string.)

In either case, rtlValue will never be either "auto" or "true", and so by all rights, isRTL should be false (the value of rtlValue == "true" on line 534) regardless of whether or not the box is checked.

Instead it's coming back as true regardless of whether or not the box is checked. Need to do some more checking on why this is.

In the meantime, a first step is to edit templates/edit-meta.tmpl at line 258 to give that checkbox a value=true so posts.go gets what it's expecting, which I have done at #106.

[joyeusenoelle]

Stream-of-consciousness comments, sorry for the spam emails if I forget myself and hit Ctrl-Enter.

In my database (Sqlite 3), which is quite small at the moment, rtl is an integer default null column. And the value WriteFreely is putting into it seems to back this up: WriteFreely places a 1 in that column when the checkbox is checked. So how is converter.NullJSONBool{sql.NullBool{Bool: isRTL, Valid: rtlValid}} generating a value of 1?

Note that Sqlite uses dynamic typing, not strict, so INTEGER columns don't have to contain integer values. I tested this by manually changing the value in rtl to {Bool: true, Valid: true}. This makes WriteFreely throw 500 errors.

So I changed it to a 0 - and my post returned to LTR.

SQL's boolean values are "truthy", like Javascript's. true, "true", and 1 are all equally True to SQL, and false, "false", and 0 are all equally False. But Go doesn't particularly like that. (Try if 1 { fmt.Println("Does this work?") } - it doesn't! Go complains.)

So what if, when we're posting the value, what we're submitting gets coerced into an integer - since by my reading we're not actually submitting a boolean value, we're submitting JSON that has a boolean value encoded in it - and then, when our database code asks for the boolean value back, SQL is saying "uh sure, that integer looks True to me"?

I'm gonna fiddle with this some more.

[joyeusenoelle]

Okay, the integer value was a red herring - as noted elsewhere, that's how MySQL and Sqlite store boolean values. But I've done some additional digging, and this time I suspect I have the answer.

In posts.go on line 611 - and this is not the only time this happens, but I think it's the salient time this happens - we use r.ParseForm() to get all the updated information out of POST. But - as noted above - if the RTL checkbox isn't checked, then there's no rtl element in POST, so r.ParseForm() doesn't know it's there, and so it doesn't add an rtl key to r.PostForm - which means that it never gets sent to the database to be updated.

In my mind, the answer is to check r.PostForm to see if the rtl key is present, and to add it if it's not.

Also in my mind, because this change is being made to the API and should support multiple clients, the best tactic is:

• If 'rtl' is not present in r.PostForm, set r.PostForm['rtl'] to sql.NullBool{nil, false}

but another tactic, keeping the current web interface in mind, is:

• If 'rtl' is not present in r.PostForm, set r.PostForm['rtl'] to sql.NullBool{false, true} (because the absence of the value from the web client means the user has unchecked the "this should be RTL" checkbox)

Is modifying PostForm the preferred approach? And if so, which tactic should we use?

And, in addition, should we update the web client to use a radio button or dropdown, to address the bug from both ends?

[thebaer]

Modifying PostForm isn't the right approach, because then we have to duplicate effort for parsing JSON (note that both operations parse data into the AuthenticatedPost instantiated earlier, before the if reqJSON block).

We should follow something similar to what we do with signups, shown in account.go (note: untested code):

1. Add a new AuthenticatedPost field, e.g. Web, defined this way:

Web bool `json:"web" schema:"web"

2. In existingPost(), do this before the app.db.UpdateOwnedPost call:

if p.Web {
  p.IsRTL.Valid = true
}

3. Add a hidden field in the post metadata editing page template:

<input type="hidden" name="web" value="true" />

[joyeusenoelle]

Okay, I'll look into how you've done it in account.go and see if I can put this together.

[thebaer]

Alright, feel free to study it, but I went ahead and implemented the fix in #110, as I didn't want you spinning your wheels on this particular task. If anything's unclear about how that fixes this, please feel free to ask and I'll be happy to explain.

Otherwise if we can, on Monday let's chat about a similarly small task you can start tackling.