Last modified: 2014-02-12 23:38:30 UTC
<form action="/foo/index.php?title=Toiminnot:Lomakemuokkaus/mr" method="get"><button type="submit" value="Lisää uusi merkitys">Lisää uusi</button></form> title is ignored. Version 2.4.2
This is a known bug - see the third item here: https://www.mediawiki.org/wiki/Extension:Semantic_Forms/Known_bugs_and_planned_features#Known_bugs Is this really of "major" severity, though? You can get around it by using "post button" instead.
At least it is in bugzilla now so more people are likely to find it. Just wondering why it isn't fixed already, seems relatively simple if you know where the code is. Feel free to tweak the fields and thanks for the work around.
Yes, it's definitely good to have it in Bugzilla as well - sorry I gave the impression that I didn't think the bug report was useful. I actually looked into fixing the issue, but it's harder than it appears - due to a problem in submitting HTML forms whose "action" contains a question marks, or something like that - I think this describes the issue: http://stackoverflow.com/questions/9058695/how-to-question-mark-in-form-action
Yaron (etal). Agreed the problem occurs, but fix is relatively simple. The problem is that MW strips ALL URL query components, including the essential "title=" In order to make this work for a form button, the "title" also has to be included as one of the "hidden" values. With the below patch, if you look at the URL, you'll still see the "title=" component in the GET URL, but it is irrelevant and ignored - could probably be eliminated. diff --git "a/git./SF_Utils-5bfa36b-left.php" "b/SF_Utils.php" Index: includes/SF_Utils.php --- "a/.git/includes/SF_Utils-5bfa36b-left.php" +++ "b/includes/SF_Utils.php" @@ -899,12 +899,14 @@ END; } $hidden_inputs = ""; if ( ! empty( $inQueryArr ) ) { - // Special handling for the buttons - query string + // Special handling for the buttons - query string and title // has to be turned into hidden inputs. if ( $inLinkType == 'button' || $inLinkType == 'post button' ) { - + $tpos = strpos( $link_url , '?title='); + if ( $tpos == true ){ + $hidden_inputs .= Html::hidden( urldecode( "title" ), urldecode( substr($link_url, $tpos + strlen('?title=') ) ) ); + } $query_components = explode( '&', http_build_query( $inQueryArr, '', '&' ) ); - foreach ( $query_components as $query_component ) { $var_and_val = explode( '=', $query_component, 2 ); if ( count( $var_and_val ) == 2 ) {
Just FYI it's not MediaWiki that is ignoring the query params.
Yeah, that's standard form behaviour for GET method forms. You have to put all query arguments into the form. And that code looks like a total ugly hack. It shouldn't be testing for just ?title= assuming that &title= doesn't exist and ignoring other query parameters. It should be generic, handling all query params in the $link_url.
Daniel (and Laxstrom) I totally agree about the ugly hack - and if there are suggestions on improvement, please submit. HOWEVER, if you look at rest of the code as it relates to button in createFormLink: 1. If I'm reading the code right, the other query params are already being handled correctly by adding them as hidden values 3 lines subsequently. 2. The only query that isn't being handled by this is the "title" param and this hack handles that (and only that) exception. 3. The "title" param is returned as part of the getTitle()->getLocalURL(). If there is a way to get ONLY the ugly "title" param, it would probably be more appropriate (I'm just not aware) 4. The hack leaves the "title=" in the URL, which is ignored, but confusing. Is it worth removing? Is there a possible security implication in leaving it visible? Should it be removed? 5. As ugly as this is, I believe it covers all conditions and solves the problem. As Yaron stated, the alternative is to use POST, but I'm assuming there is a reason somewhere (in my case, legacy apps), that GET is preferred. I'd appreciate any improvements!!!!!
Created attachment 11968 [details] Ugly hack Patch Admittedly ugly hack - that works. Any better suggestions?
(In reply to comment #7) > Daniel (and Laxstrom) > > I totally agree about the ugly hack - and if there are suggestions on > improvement, please submit. HOWEVER, if you look at rest of the code as it > relates to button in createFormLink: > > 1. If I'm reading the code right, the other query params are already being > handled correctly by adding them as hidden values 3 lines subsequently. Those are query parameters added by things like |query string= inside the form definition. Any other query params inside link_url are not being handled. > 2. The only query that isn't being handled by this is the "title" param > and this hack handles that (and only that) exception. > 3. The "title" param is returned as part of the getTitle()->getLocalURL(). > If there is a way to get ONLY the ugly "title" param, it would probably > be more appropriate (I'm just not aware) getLocalURL is hookable. Other extensions may make it return urls with other query params and in a different order. Though just for the record, you can get the same title text out of Title that getLocalURL uses to make the title param. > 4. The hack leaves the "title=" in the URL, which is ignored, but confusing. > Is it worth removing? Is there a possible security implication in leaving > it visible? Should it be removed? Removing the params that have been moved into inputs is the correct thing to do. > 5. As ugly as this is, I believe it covers all conditions and solves the > problem. As Yaron stated, the alternative is to use POST, but I'm > assuming there is a reason somewhere (in my case, legacy apps), that > GET is preferred. False dilemma, neither POST nor hacks of this level are necessary. It is not that hard to write some sane code that actually handles this situation correctly without being a hack. - Don't look for ?title. Instead break apart the url and it's query string. - Add hidden inputs for all the params you find in the query string. This can be done in the same way that the code is already doing to handle the user provided |query string= data. In fact you could probably merge the queries and avoid code duplication. - Don't do this inside of the `if ( ! empty( $inQueryArr ) ) {` conditional. Doing this inside of there leads to the possibility of link_url not being properly handled and the bug coming back in cases like extra user specified query strings not being provided. Frankly createFormLink's generation of the $link_url in the first place needs some fixes too. It shouldn't be appending to the url like that. That stuff should go into the title (or rather special page subtitle) before getLocalURL is called.
