"Create pages with form" and sfEditFormPreloadText
Closed, ResolvedPublic

Description

Author: smw

Description:
sfEditFormPreloadText hooks don't seem to be getting called when pages are created via the Create_pages_with_form property.

MW: 1.21.1
SMW: 1.8.0.5
SF: 2.6

Steps to reproduce:

  1. Make a sfEditFormPreloadText extension.
  2. Demonstrate that it works correctly by making a couple hundred articles using it in the conventional fashion.
  3. Insert subtle logging statement like die("I'm getting called!"); into first line of function.
  4. Add [[Create pages with form::<formname>]] statement to Property:Some_property.
  5. Run jobs and note that pages are getting created via the form but the preload function is not getting called or else the script would die, obv.

Version: unspecified
Severity: normal

bzimport set Reference to bz56569.
bzimport created this task.Via LegacyNov 4 2013, 5:01 PM
Yaron_Koren added a comment.Via ConduitNov 4 2013, 5:20 PM

Re-assigning to Stephan - this seems more like your territory. What do you think?

bzimport added a comment.Via ConduitNov 4 2013, 11:45 PM

smw wrote:

Looking at the actual code, there's clearly no wfRunHooks call along the code path invoked by this function, so it's not a matter of a bug.

It would seem to be a fairly simple problem to solve -- just add in the wfRunHooks call before the call to formHTML() in... uh... whatever function I saw that was invoked in response to the Creates_pages_with_form property.

However, my nice little attempted patch didn't work. And furthermore, replacing the variable with 'WHEEEEEEEEEEEEEE!!!!' still led to pages created with e.g. {{Form_Template|param1=arg}} rather than the expected 'WHEEEEEEEEEEEEEE!!!!' (or even containing that string at all... I thought it might be thrown in free text or something, but it wasn't).

In other words, there's something going on in formHTML() that I hadn't anticipated, and I don't have the patience or knowledge to tinker with that function :/

bzimport added a comment.Via ConduitNov 4 2013, 11:47 PM

smw wrote:

Not that you don't know any of that, but I thought I'd mention it to show I'm making a good-faith effort to contribute and not just whining :)

gerritbot added a comment.Via ConduitNov 5 2013, 9:05 PM

Change 93813 had a related patch set uploaded by Foxtrott:
fix bug 56569 ("Create pages with form" and sfEditFormPreloadText)

https://gerrit.wikimedia.org/r/93813

Foxtrott added a comment.Via ConduitNov 5 2013, 9:07 PM

Nathan, could you try, if the patched version works as expected?

gerritbot added a comment.Via ConduitNov 5 2013, 9:17 PM

Change 93813 merged by Yaron Koren:
fix bug 56569 ("Create pages with form" and sfEditFormPreloadText)

https://gerrit.wikimedia.org/r/93813

bzimport added a comment.Via ConduitNov 5 2013, 9:42 PM

smw wrote:

Sorry, folks, it doesn't work for me.

It doesn't seem as though SFFormLinker::createLinkedPage() or SFCreatePageJob go through the API, but that could just be my lack of familiarity with the code and using a glorified text editor instead of a real IDE :/

gerritbot added a comment.Via ConduitNov 5 2013, 10:14 PM

Change 93865 had a related patch set uploaded by Foxtrott:
fix bug 56569 ("Create pages with form" and sfEditFormPreloadText)

https://gerrit.wikimedia.org/r/93865

Foxtrott added a comment.Via ConduitNov 5 2013, 10:16 PM

Seems I fixed a different bug with the first patch. Could you try the new patch, please?

bzimport added a comment.Via ConduitNov 5 2013, 10:31 PM

smw wrote:

Still not working. For the record, that's very close to the patch I attempted and mentioned briefly in comment #2. formHTML() hates my guts?

FWIW, the pages created for this test previously existed but were deleted (since they didn't contain the desired data). Just in case the presence of a deleted article is a possibly confounding issue...

bzimport added a comment.Via ConduitNov 6 2013, 12:12 AM

smw wrote:

Actually, just noticed that Change 93813 seems to break my extension -- from what I can tell, the $preloadContent variable is populated correctly but never inserted. Never utilized by formHTML()?

bzimport added a comment.Via ConduitNov 20 2013, 2:16 PM

smw wrote:

To reiterate, I think this commit may break all sfEditFormPreloadText extensions.

bzimport added a comment.Via ConduitJan 8 2014, 1:03 AM

smw wrote:

A new sfEditFormPreloadText extension doesn't work either. This is with the latest commit (#c1b52276c2492a51 or so).

bzimport added a comment.Via ConduitMay 9 2014, 8:13 PM

smw wrote:

Still doesn't work. Am I the only person who uses sfEditFormPreloadText?

Maybe what I should do is turn my extensions into regular text preloading extensions, create and save the page, *then* edit it with the form.

bzimport added a comment.Via ConduitMay 12 2014, 2:39 AM

cariaso wrote:

You are not the only one. An important feature of SNPedia is impacted by this.

This is a regression of
https://bugzilla.wikimedia.org/show_bug.cgi?id=47150

bzimport added a comment.Via ConduitMay 12 2014, 7:41 PM

smw wrote:

Thanks for the information, cariaso. I'm glad to have some more of the context to flesh out this cluster of issues.

bzimport added a comment.Via ConduitAug 4 2014, 4:10 AM

cariaso wrote:

I spent some time looking at this, and am confident that the function which is hooked to 'sfEditFormPreloadText'

is being called for existing pages

is _not_ being called for non existent pages

bzimport added a comment.Via ConduitAug 4 2014, 4:37 AM

cariaso wrote:

The problem originates near
SF_FormEditAction.php Line 157

static function displayForm( $action, $article ) {

	        // @todo: This looks like bad code. If we can't find a form, we
		// should be showing an informative error page rather than
		// making it look like an edit form page does not exist.
                $title = $article->getTitle();
		$form_names = SFFormLinker::getDefaultFormsForPage( $title );
                if ( count( $form_names ) == 0 ) {
	                return true;
		}

notice the @todo comment, suggesting this code was never quite finished. returning true here, bails out before the

SFFormEdit::printForm( $form_name, $page_name );

a few lines further down in the code.

for pages which exist, they eventually get down to the hook code via this route.

#0 [internal function]: prefillForm(string, Title, Title)
#1 /var/www/html/includes/Hooks.php(206): call_user_func_array(string, array)
#2 /var/www/html/includes/GlobalFunctions.php(4004): Hooks::run(string, array, NULL)
#3 /var/www/html/extensions/SemanticForms/includes/SF_AutoeditAPI.php(834): wfRunHooks(string, array)
#4 /var/www/html/extensions/SemanticForms/includes/SF_AutoeditAPI.php(116): SFAutoeditAPI->doAction()
#5 /var/www/html/extensions/SemanticForms/specials/SF_FormEdit.php(92): SFAutoeditAPI->execute()
#6 /var/www/html/extensions/SemanticForms/includes/SF_FormEditAction.php(173): SFFormEdit::printForm(string, string)
#7 /var/www/html/extensions/SemanticForms/includes/SF_FormEditAction.php(27): SFFormEditAction::displayForm(SFFormEditAction, Article)
#8 /var/www/html/includes/Wiki.php(428): SFFormEditAction->show()
#9 /var/www/html/includes/Wiki.php(292): MediaWiki->performAction(Article, Title)
#10 /var/www/html/includes/Wiki.php(588): MediaWiki->performRequest()
#11 /var/www/html/includes/Wiki.php(447): MediaWiki->main()
#12 /var/www/html/index.php(46): MediaWiki->run()
#13 {main}

however empty pages bail out early, and never get the chance to prepopulate their page text.

gerritbot added a comment.Via ConduitSep 11 2014, 8:57 AM

Change 93865 had a related patch set uploaded by Qgil:
fix bug 56569 ("Create pages with form" and sfEditFormPreloadText)

https://gerrit.wikimedia.org/r/93865

gerritbot added a comment.Via ConduitSep 11 2014, 2:37 PM

Change 93865 merged by Yaron Koren:
fix bug 56569 ("Create pages with form" and sfEditFormPreloadText)

https://gerrit.wikimedia.org/r/93865

bzimport added a comment.Via ConduitSep 11 2014, 3:04 PM

smw wrote:

As far as I can tell, this still breaks all sfEditFormPreloadText extensions.

Yaron_Koren closed this task as "Resolved".Via WebFeb 26 2015, 7:24 PM
Yaron_Koren set Security to None.

Add Comment