Clarifying add_theme_support() inline docs for 'html5'

[jond3r]

In #26697 was introduced support for HTML5 markup in image galleries. To enable this you would have to write:

add_theme_support( 'html5', array( 'gallery' ) );

Previously there were just three so-called 'types' of HTML5 support in core: 'comment-list', 'comment-form', 'search-form'.
So now there is one more: 'gallery'!

This has pinpointed a weakness (or asymmetry if you like) in the current API for theme support. Though discouraged (ticket:26697#comment:12), it is currently possible and legitimate to use:

add_theme_support( 'html5' );

which adds support for the three types 'comment-list', 'comment-form' and 'search-form', but not the new type 'gallery'!
Thus, as the code stands today 'gallery' is treated as a second-class citizen, which it does not deserve!

Another manifestation of the current asymmetry of the API is that remove_theme_support( 'html5' ) removes all previously added types, but add_theme_support( 'html5' ) only adds back three of them.

To add some sanity back to the theme support API, three possible roads are outlined:

1. Keep the current code as is, for preserved backwards compatibility, but add some comment to the code and an additional assertion for clarity to the theme-support test. This is provided in the attached patch no. 1.

2. Add 'gallery' to the default list of types for preserved symmetry but broken back-compat, with some additional test assertion. This is patch no. 2.

3. Remove the possibility to use add_theme_support( 'html5' ) altogether, which breaks back-compat this time, but is future proof as other types are added. It also restores symmetry of the API, albeit in another form. Adjust the unit tests accordingly. This is patch no. 3.

I do not have very strong feelings about which road is better. No. 1 is safest, but does not provide much improvement. Patch no. 2 was rejected in ticket:26697#comment:12, and will realistically not be accepted as it is not future proof and breaks back-compat in a silent way: gallery styling might stop working for themes or plugins using add_theme_support( 'html5' ). Patch no. 3 is 100% future-proof, but usage of add_theme_support( 'html5' ) will trigger a doing-it-wrong error. It is somewhat 'boring' though, as something is taken away, and a longer api-call must be made to achieve the same thing as before.

Much of the current behavior was added in #24932.

[SergeyBiryukov]

Though discouraged (ticket:26697#comment:12), it is currently possible and legitimate to use:
add_theme_support( 'html5' );
which adds support for the three types 'comment-list', 'comment-form' and 'search-form', but not the new type 'gallery'! Thus, as the code stands today 'gallery' is treated as a second-class citizen, which it does not deserve!

That list is just for backwards compatibility, new items should not be added there.

27278.4.diff​ adds a _doing_it_wrong() call and a "Back compat" comment, per comment:12:ticket:26697. I think that's all we need here. We didn't do that originally, as the consensus in comment:12:ticket:24932 was that _doing_it_wrong() was not necessary until we actually had a new markup in some other function. Now that we've added it to gallery_shortcode(), let's do this.

[jond3r]

Per Sergey's suggestion, but adding @return-tag to doc block and updated unit tests

[nathanrice]

Option 1 seems sensible to me. It doesn't relegate gallery to any non-preferred status, it's just a fallback for people who implemented html5 support before gallery support existed. If all the documentation recommends being explicit with what elements you want on, use of html5 without explicit support will go way down.

This preserves backward compat without breaking sites.

Removing the ability to use html5 by itself isn't "future proof as other types are added" any more than option 1 is. All it does is forces unnecessary failures with the only real benefit being the removal of some code.

As far as I'm concerned, this doesn't even warrant a _doing_it_wrong trigger. Themes (for the most part) don't exactly have a way of pushing updates out to users.

[obenland]

Yes, door 4 please.

[jond3r]

#27292 suggests a new html5 type: 'caption'

[jond3r]

The general message here seems to be: "Change as little as possible". Still I think it would be valuable to have some comment in the code to indicate what is considered as deprecated usage (some devs might not read the codex).

The attached .6 patch is essentially a modified version of .1 that tries to better adhere to the documentation standards (replacing 'void' with 'mixed' in the @return line). The unit tests are also not touched.

Edit: I raised the issue of using void in mixed type @return here: comment:19:ticket:27098

[jond3r]

A minimal patch. Only adding comment.

[SergeyBiryukov]

I don't really see the benefit of adding a @return value here.

This function doesn't have a consistent return value (nor do I think it should), so why would anyone check it? The _doing_it_wrong() message is pretty clear.

Let's just add a comment from 27278.6.diff​.

[jond3r]

Replying to SergeyBiryukov:

I don't really see the benefit of adding a @return value here.

I agree to some extent. I think it is best to have a consistent return type, bool e.g., but that should have been thought of at design time.

[jond3r]

A modified minimal patch.

[jond3r]

Patch .7 restores the use of @return void|bool as I think this is accepted usage of void, based on for example [27575].

The @return line can be skipped if deemed unnecessary (though I think it should be added).

[jond3r]

Just a refresh

[jond3r]

Changed the title to more accurately describe the current patch.

[DrewAPicture]

In 27879:

Clarify inline documentation for the 'html5' case and PHPDoc return in add_theme_support().

Props jond3r.
Fixes #27278.