Support placeholder, label, value attributes in removal reasons

[molenzwiebel]

As per this feature request on the sub, this PR adds support for the placeholder, label and value attributes in removal reasons.

The linked post explains the reasoning behind allowing the attributes. Internally, the default removal reason already had a placeholder but it was never parsed because it was missing from the whitelist.

I've confirmed that the label/value attribute on <option> tags works as expected. I've also confirmed that an empty textarea with a placeholder will not take that placeholder as value or other weirdness.

[creesch]

Oh boy, we have been holding back on removal reason html parsing things due to #118 probably okay here I suppose but good to know for the future.

@Geo1088 what do you think?

[eritbh]

Was mostly holding back from implementing the placeholder thing just because I didn't want to put in the effort before tackling #118. If somebody else has already written it in though, I don't see an issue with including it now, especially since we're clearly in no rush to take care of #118.

Also related: https://www.reddit.com/r/toolbox/comments/er30rx/issues_with_placeholder_text_in_multiline_inputs/

[eritbh]

Is this PR tested? I don't see anything here that would make placeholder work as expected, since the value of a blank <textarea placeholder="..."> is still an empty string IIRC.

[molenzwiebel]

As per my initial message,

I've also confirmed that an empty textarea with a placeholder will not take that placeholder as value

If you want to be consistent with the entirety of the web, then placeholders should act as hints for the user and not as default values. With that in mind, treating empty textareas as empty even if they have a placeholder attribute seems to be the right call.

[eritbh]

Must've missed that, my bad. The feature request I linked asks for placeholder to be used as a default value used if no text is typed, which seems like a reasonable request to me. I'm not sure what good whitelisting it does otherwise - it'd be a hint for what to type there, but isn't that normally evident by the rest of the message?

I know this doesn't really fit the semantic purpose of placeholder, but I can't think of a better way to handle it here.

[molenzwiebel]

It'd be fairly easy to add, but I'm worried it may cause confusion if gray text people are trained to see as suggestion/placeholder suddenly gets posted as part of the message.

One idea could be to use css to make the placeholder have the same color as normal text. That way we don't need to manually implement placeholder logic and at the same time should cause less confusion for users.

Should only be a couple lines of css/code to get that working.

[eritbh]

Yeah that seems reasonable to me. This should do it as far as CSS goes, ensuring the color is consistent with RES nightmode and working with <textarea> as well as <input type="text">:

.mod-toolbox-rd .reason-popup ::placeholder {
  color: inherit;
}

[creesch]

A bit more complex but would also allow people to edit the placeholder is to simply copy the placeholder to value. That way we don't have to rely on CSS (and possibly subreddit CSS fucking with it accidentially).

[molenzwiebel]

The problem with copying the placeholder to the value is that it becomes ambiguous when it should be reset to the placeholder. If we reset it when the field is empty, it'll interfere with people that want a different message entirely. If we reset it on page load, it'll interfere with the persisting of the contents, etc, etc.

I've committed the placeholder hack for now. Looks like this:

[eritbh]

This works for me. We'll find a more semantic solution to this whenever we get around to #118. Thanks again, and sorry it took me so long to get around to reviewing these pulls.