Change Details

The following issues were found in Page Forms by @Bawolff earlier this year. I'm putting them all in one place for convenience. [] "PF_CreateForm.js" line 4 - The other options ajax is totally broken on Special:CreateForm if the url has a #target fragment in it, like http://localhost:8080/wiki/Special:CreateForm#foo [] Parts of this extension don't follow MW code style conventions. This doesn't really matter, and its totally reasonable for a third party developed extension to deviate from MW style conventions. However, if you want it consistent with current MW style conventions, you may want to consider enabling phpcs. [] Some uses of global state seem to assume only one form related page will be parsed in a given request. This is probably true, but not considered a best practise assumption. It would be better to store such state in the Parser object probably. [] "PF_FormUtils.php" line 233 cancelLinkHTML - Use of $wgTitle is strongly discouraged by modern MediaWiki conventions. The calling function should pass in a title instead. [] "PF_FormUtils.php" line 235 cancelLinkHTML - It would probably be better to detect MSIE in javascript, since then there is no chance of cache pollution (In the context of a special page, there should be no caching, so its unlikely to be an issue, but client side detection where possible is better). [] "PF_FormUtils.php" line 235 cancelLinkHTML - Its preferred in modern MediaWiki to not use Javascript: URLs for Javascript, but instead attach the javascript from a RL module. Javascript: urls would break if we ever manage to adopt Content-Security-Policy (In fairness, unlikely to happen any time soon). [] "PF_Hooks.php" line 45 - Should use $wgExtensionAssetPath [] "PF_ParserFunctions.php" line 155 - no guarantee that global state corresponds to right page, should be done via RL. [X] "PF_FormUtils.php" line 54 - maxlength=200. Most MW has moved to using jquery.byteLimit so that its exactly 255 UTF-8 bytes. [] "PF_FormEdit.php" line 145 - $targetTitle might be null. e.g. Special:FormEdit?form=Foo&target=%20 [] Somewhere (not sure where) {{Special:RunQuery}} is calling the parser recurively when its being transcluded. You should use $wgParser->getFreshParser() in such situations to avoid UNIQ-... tags. [] [Not clear on the impact] "PF_AutocompleteAPI.php" line 316 - $whereStr = "$baseCargoTable.$baseCargoField = \"$baseValue\""; - The lack of escaping $baseValue seems concerning here. My understanding is that its processed by Cargo as untrusted input so this doesn't represent a security issue, but nonetheless, this seems kind of sketchy. [] "PF_CreatePageJob.php" line 44 - Should maybe use RequestContext::importScopedSession() instead. [] "PageForms.js" line 1265 - async: false is bad... [] Previewing direct from url (e.g. action=edit&preview=yes) a form page does not show the form.

The following issues were found in Page Forms by @Bawolff earlier this year. I'm putting them all in one place for convenience. [] "PF_CreateForm.js" line 4 - The other options ajax is totally broken on Special:CreateForm if the url has a #target fragment in it, like http://localhost:8080/wiki/Special:CreateForm#foo [] Parts of this extension don't follow MW code style conventions. This doesn't really matter, and its totally reasonable for a third party developed extension to deviate from MW style conventions. However, if you want it consistent with current MW style conventions, you may want to consider enabling phpcs. [] Some uses of global state seem to assume only one form related page will be parsed in a given request. This is probably true, but not considered a best practise assumption. It would be better to store such state in the Parser object probably. [] "PF_FormUtils.php" line 233 cancelLinkHTML - Use of $wgTitle is strongly discouraged by modern MediaWiki conventions. The calling function should pass in a title instead. [X] "PF_FormUtils.php" line 235 cancelLinkHTML - It would probably be better to detect MSIE in javascript, since then there is no chance of cache pollution (In the context of a special page, there should be no caching, so its unlikely to be an issue, but client side detection where possible is better). [X] "PF_FormUtils.php" line 235 cancelLinkHTML - Its preferred in modern MediaWiki to not use Javascript: URLs for Javascript, but instead attach the javascript from a RL module. Javascript: urls would break if we ever manage to adopt Content-Security-Policy (In fairness, unlikely to happen any time soon). [] "PF_Hooks.php" line 45 - Should use $wgExtensionAssetPath [] "PF_ParserFunctions.php" line 155 - no guarantee that global state corresponds to right page, should be done via RL. [X] "PF_FormUtils.php" line 54 - maxlength=200. Most MW has moved to using jquery.byteLimit so that its exactly 255 UTF-8 bytes. [] "PF_FormEdit.php" line 145 - $targetTitle might be null. e.g. Special:FormEdit?form=Foo&target=%20 [] Somewhere (not sure where) {{Special:RunQuery}} is calling the parser recurively when its being transcluded. You should use $wgParser->getFreshParser() in such situations to avoid UNIQ-... tags. [] [Not clear on the impact] "PF_AutocompleteAPI.php" line 316 - $whereStr = "$baseCargoTable.$baseCargoField = \"$baseValue\""; - The lack of escaping $baseValue seems concerning here. My understanding is that its processed by Cargo as untrusted input so this doesn't represent a security issue, but nonetheless, this seems kind of sketchy. [] "PF_CreatePageJob.php" line 44 - Should maybe use RequestContext::importScopedSession() instead. [] "PageForms.js" line 1265 - async: false is bad... [] Previewing direct from url (e.g. action=edit&preview=yes) a form page does not show the form.

The following issues were found in Page Forms by @Bawolff earlier this year. I'm putting them all in one place for convenience. [] "PF_CreateForm.js" line 4 - The other options ajax is totally broken on Special:CreateForm if the url has a #target fragment in it, like http://localhost:8080/wiki/Special:CreateForm#foo [] Parts of this extension don't follow MW code style conventions. This doesn't really matter, and its totally reasonable for a third party developed extension to deviate from MW style conventions. However, if you want it consistent with current MW style conventions, you may want to consider enabling phpcs. [] Some uses of global state seem to assume only one form related page will be parsed in a given request. This is probably true, but not considered a best practise assumption. It would be better to store such state in the Parser object probably. [] "PF_FormUtils.php" line 233 cancelLinkHTML - Use of $wgTitle is strongly discouraged by modern MediaWiki conventions. The calling function should pass in a title instead. [X] "PF_FormUtils.php" line 235 cancelLinkHTML - It would probably be better to detect MSIE in javascript, since then there is no chance of cache pollution (In the context of a special page, there should be no caching, so its unlikely to be an issue, but client side detection where possible is better). [X] "PF_FormUtils.php" line 235 cancelLinkHTML - Its preferred in modern MediaWiki to not use Javascript: URLs for Javascript, but instead attach the javascript from a RL module. Javascript: urls would break if we ever manage to adopt Content-Security-Policy (In fairness, unlikely to happen any time soon). [] "PF_Hooks.php" line 45 - Should use $wgExtensionAssetPath [] "PF_ParserFunctions.php" line 155 - no guarantee that global state corresponds to right page, should be done via RL. [X] "PF_FormUtils.php" line 54 - maxlength=200. Most MW has moved to using jquery.byteLimit so that its exactly 255 UTF-8 bytes. [] "PF_FormEdit.php" line 145 - $targetTitle might be null. e.g. Special:FormEdit?form=Foo&target=%20 [] Somewhere (not sure where) {{Special:RunQuery}} is calling the parser recurively when its being transcluded. You should use $wgParser->getFreshParser() in such situations to avoid UNIQ-... tags. [] [Not clear on the impact] "PF_AutocompleteAPI.php" line 316 - $whereStr = "$baseCargoTable.$baseCargoField = \"$baseValue\""; - The lack of escaping $baseValue seems concerning here. My understanding is that its processed by Cargo as untrusted input so this doesn't represent a security issue, but nonetheless, this seems kind of sketchy. [] "PF_CreatePageJob.php" line 44 - Should maybe use RequestContext::importScopedSession() instead. [] "PageForms.js" line 1265 - async: false is bad... [] Previewing direct from url (e.g. action=edit&preview=yes) a form page does not show the form.