[3.0] Theme split (wave 4, part 4) — close the theme picker's form - #9388
Open
albertlast wants to merge 1 commit into
Open
[3.0] Theme split (wave 4, part 4) — close the theme picker's form#9388albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
template_pick() opens a form and never closes it, so where it ends is left to the browser's error recovery rather than the markup. The session variable, the security token and the member id are echoed inside the loop that runs once for the current theme and once for the rest, so each of them is sent twice. Closes the form after the loop, and echoes one set of hidden fields. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Part of the split of #7933, wave 4 part 4. The theme picker,
template_pick()inThemes.template.php.Two things wrong with it, both visible in the served page:
The form is never closed.
<form action="…?action=themechooser" …>is openedand there is no
</form>, so where the form ends is left to the browser's errorrecovery rather than to the markup.
The hidden fields are echoed twice. They sit inside the
for ($i = 0; $i < 2; $i++)loop that draws the current theme in one pass and everything else in theother, so the session variable, the security token and
uare each emitted onceper pass.
Testing
Measured on the rendered page at
?action=themechooser;u=1, counting the fieldsof the picker form:
release-3.0pick-thtoken ×2,u×2Both
save[…]buttons and both theme sections are still inside the formafterwards, and submitting it still saves the theme.
To submit it at all I first needed #9387 —
ThemeChooserfatals on an unimportedclass before it gets as far as saving. That one is a separate PR since it is a
Sources/bug rather than a template one.The one thing I would flag: the browser does not, in practice, swallow the footer
into the unclosed form — it closes it at the enclosing element. So this is invalid
markup and duplicated fields rather than anything user-visible today.
Issues References (Fixes|Related|Closes)
Related to #7933