Retrieve "class" and "subPage" from form attributes even when urlencoded

[christianwach]

Overview

Fixes problems associated with Contribution Page forms and Event Management forms as described in this comment by @kcristiano.

The issue has appeared since #13043 because the existing code relied on URLs not being URL-endcoded to tease out the component for the active tab. Please see the discussion on the linked issue for further details.

Before

The component is misidentified by parsing the action attribute of the form object using basename(), but fails because the q variable is (properly) URL-encoded since #13043.

After

The component is correctly identified by parsing the action attribute of the form object using basename() by not assuming that the URL is in its raw state. The new code URL-decodes the action URL when needed before teasing out the component name.

Comments

This only fails in WordPress because Drupal uses clean URLs by default and WordPress can never use clean URLs in wp-admin because everything is routed through wp-admin/admin.php.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@christianwach we seem to be repeating the same code 4 places - maybe add a function on CRM_Utils_Request class or CRM_Utils_String (I think the former has the url() function) & call that?

[christianwach]

@eileenmcnaughton Fair point. I had considered this but wasn't sure where to locate such a method - thanks for the suggestion.

[eileenmcnaughton]

@xurizaemon @totten @seamuslee001 this needs a quick security vetting I think

[kcristiano]

@eileenmcnaughton I have tested this in D7 and WP and it solves the issue described on WP while not affecting D7 at all.

Your comments make sense. So the only issue is do we have time to get this in to 5.8 or do we need to punt this PR and the earlier committed one #13043 to master/5.9

[seamuslee001]

So looking at this

before
no validation is done on the type of object that action is passed. However only specific types of actions are used and there seems to be solid enough ensuring that anything nasty can't really get in

after
Same as before

Agree with Eileen on the code cleanup routine. @christianwach @kcristiano if the code duplication issue can be resolve then i am happy for this to got into 5.8 given the other PR is in 5.8. Noting 5.8 is the December release so we have ~2 weeks left in RC

[alifrumin]

@kcristiano I'm impressed that you caught this and by your and @christianwach efforts to get this fixed for 5.8.

Let me know if you would like me to test :).

[kcristiano]

@alifrumin we love testing so please do and Thank You

[christianwach]

Closed in favour of #13099