Last modified: 2013-06-10 03:24:41 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T24037, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 22037 - Advanced Formatting for Query Tables
Advanced Formatting for Query Tables
Status: REOPENED
Product: MediaWiki extensions
Classification: Unclassified
Semantic MediaWiki (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Markus Krötzsch
http://wiki.montcopa.org/TestWiki/ind...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-07 03:54 UTC by Jack D. Pond
Modified: 2013-06-10 03:24 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Patches required to implement functionality (7.89 KB, patch)
2010-01-07 03:54 UTC, Jack D. Pond
Details
Updated to r60790 adjustments and added other params for Special:Ask (8.31 KB, patch)
2010-01-08 21:53 UTC, Jack D. Pond
Details
introtemplate and outrotemplate functionality + params passing fix to Special:Ask (2.88 KB, application/octet-stream)
2010-01-20 16:45 UTC, Jack D. Pond
Details
introtemplate and outrotemplate functionality + params passing fix to Special:Ask (2.87 KB, application/octet-stream)
2010-01-20 16:51 UTC, Jack D. Pond
Details
introtemplate and outrotemplate functionality + params passing fix to Special:Ask - patched to current HEAD (2.75 KB, patch)
2010-01-20 18:30 UTC, Jack D. Pond
Details
introtemplate and outrotemplate functionality + params passing fix to Special:Ask - patched to current HEAD (2.49 KB, patch)
2010-05-07 15:09 UTC, Jack D. Pond
Details
Advanced Formatting and "further results" fix (8.33 KB, patch)
2010-06-29 14:34 UTC, Jack D. Pond
Details
Advanced Formatting and "further results" fix - format/version fixes (4.00 KB, patch)
2010-06-29 19:13 UTC, Jack D. Pond
Details

Description Jack D. Pond 2010-01-07 03:54:51 UTC
Created attachment 6931 [details]
Patches required to implement functionality

Enable template-based formatting for Query Tables.

Some of the uses of this advanced technique include: 

* Set a cell to hyperlink to the article, instead of using the article name in a separate column. 
* Concatenate or Adjust Fields - Allow columns to display concatenated properties, or adjust/modify for a specific display purpose. 
* Column/Cell Level Text Alignment - Changing the text alignment in a cell or column. 
* Define whether or not a column is sortable. 
* Highlight Cells (via formatting) based on content. 
* Define Column Widths to enhance readability. 
* Define Table Width (allow left/right scroll ability instead of "scrunching" for very wide tables). 

A complete description of the proposed functionality and documentation of how-to can be found at:
http://wiki.montcopa.org/TestWiki/index.php?title=Advanced_Table_Formatting_for_SMW_Queries

The entire functionality can be provided by modifying only a single script, but can also be done in a way that is 100% backward compatible and has little to no performance impact if not utilized.

A working version of this functionality can be seen at:

http://wiki.montcopa.org/TestWiki/index.php?title=JDPTestPage

This approach is also required to allow customized query output that is consistent between an in-line query and the "...further results" (Special:Ask) functionality.
Comment 1 Jack D. Pond 2010-01-07 17:00:38 UTC
Yoran made a suggestion that this might be an issue on more than format=table, and possibly should be included as a generic feature for format=template rather than just format=table (e.g., allow headertemplate for all template types).  I think this is a good idea, but of course I wish I'd thought of it before I implemented in SMW_QP_Table.php.

In looking at it last night, this would take some work, and impact all of the SMW_QP_*.php files.

The reason we originally explored this extended functionality is because we were creating headers (and footers) for tables and using the format=template in the articles/pages directly.  HOWEVER, this really fell apart when you reach the "...further results", formatting does not carry into the Special:Ask, which it will do under proposed changes.

Another thing we discovered is that parameters passed in the original query were also not forwarded in the "...further results" link and in many, if not all cases, the special results printers returned HTML (to reduce reparsing), while allowing templates pretty much requires WIKI returns.

Finally, the use of a custom headertemplate pretty much mandates that you also have a custom footertemplate (think of the header/footer as an envelope for the content).  If you deal only with tables, the footer can always be simply be a terminate table ("|}"), but with other types of headers, not so much.

With all of these factors combined it was easier just to deal with format=table[broadtable] rather than all the possible permutations.

If the consensus it that we should do for format=template, it's going to take a lot more work, but I'll abide by the decision and make it happen (ughh).
Comment 2 Jack D. Pond 2010-01-07 17:09:47 UTC
Other factors for consideration:

1. During the development, realized that parameters passed to query, were not being forwarded in the "...further results" link.  This can cause problems.  Should this be addressed on a more generic basis, e.g., automatically forward ALL params for all format= types?
2. There were several XSS/Injection vectors in this code, that I think I closed.  Should a more thorough review be done?
3. In the custom SMWResultPrinter (SMWTableResultPrinter) one of the initiating parameters is $outputmode, which in Special:Ask is set to SMW_OUTPUT_HTML.  To allow wiki syntax templates, I overrode this requested format by setting $this->isHTML to false when a template is used, which seems to work fine and re-parses for wiki code.  Note:  Without one or both of the template parameters, the returned result is HTML and no re-parsing is set or done.
Comment 3 Jack D. Pond 2010-01-08 21:53:13 UTC
Created attachment 6941 [details]
Updated to r60790 adjustments and added other params for Special:Ask
Comment 4 Jack D. Pond 2010-01-08 21:57:48 UTC
Updated r60790 adjustments made by Markus.

format=table now uses (and passes to Special:Ask) the following additional params:

[new]
headertemplate
fixedwidth
[existing]
template
intro
outro
link
columns
userparm
Comment 5 Markus Krötzsch 2010-01-10 19:28:52 UTC
Thanks, the code will certainly be considered for inclusion in SMW. An alternative to speed up publication would be to include it into Semantic Result Formats (SRF) first (it is possible that formats included there replace SMW formats). This would also get more people to test it before going to SMW.

I am slightly concerned here that more and more functionality makes SMW larger and larger, making a code review more difficult and less likely (Bug 21602). This is especially true for components of the code that are definitely needed to use SMW, as is the case for the table printer. This might be another reason to move the extended table into SRF.
Comment 6 Jack D. Pond 2010-01-19 22:58:08 UTC
(In reply to comment #5)

Agreed, but for consideration:
a) Is not executed unless features are used (e.g., parameters specified)
b) Provide extensive additional formatting capability sorely needed in many applications (including many in the County - only way to get developers to use SMW)
c) The ...further results stuff isn't working anyway for complex/template driven queries, so would need to be fixed some other way.

and most importantly:

d)   "I am a bear of very little brains" - Winnie the Pooh.  Overriding the table function in SRF is beyond my capability, and creating a new "custom" table format is stretching it - not to mention the amount of redundant code that would have to be maintained in this effective "branch" of the table code.

However, do as you see fit - I'll try and live the results.
Comment 7 Jack D. Pond 2010-01-20 16:45:39 UTC
Created attachment 6985 [details]
introtemplate and outrotemplate functionality + params passing fix to Special:Ask

If at first you don't succeed. . . .
Yaron really helped me out with this one.  His suggestion is to move the code from SMW_QP_Table.php to SMW_QP_List.php and use two new parameters (basically table header and table footer) as follows:

introtemplate - reference to template inserted before query returns
outrotemplate - reference to template inserted after all query returns

These two parameters extend the functionality of the format=template query.

Example:
{{#ask:
 [[Category:Active Participants List]] [[Client Status::Pending]] 
 |?Client Last Name
 |?Client First Name
 |?Client MI
 |?Client Status
 |?Client Referral Date
 | format=template
 | introtemplate=MHUserHdr
 | template=MHUserRow
 | outrotemplate=CustomOutro
 | link=none
 | limit=3
}}

In this case the introtemplate initiates a custom table - with header columns ( "{| . . ." ) and the outrotempate closes the custom table ( "|}" ).

Advantage to this approach is far simpler (requires only 4 lines of code).

Additionally this fixes the missing parameters which should have been passed to the Special:Ask via the "...further results" link - which keeps the returned query list consistent with the original query.
Comment 8 Jack D. Pond 2010-01-20 16:51:57 UTC
Created attachment 6986 [details]
introtemplate and outrotemplate functionality + params passing fix to Special:Ask

AAAARRRRGGH - attached wrong patch.  This is the right patch
Comment 9 Jack D. Pond 2010-01-20 18:30:23 UTC
Created attachment 6988 [details]
introtemplate and outrotemplate functionality + params passing fix to Special:Ask - patched to current HEAD

Patched to current head version and removed $mLinkParm - this parameter needs to be addressed for other SMW_QP_*.php classes, but for right now, just complicates a very specific purpose patch.
Comment 10 Ulli 2010-01-23 09:15:10 UTC
If you would like to hear the opinion from an end user: I am very glad about "Advanced Table formatting" and will try to integrate the patch for "further results". Although it seems to be hard work to solve the problems with "formatting on further results" it would be a great additional value. Anyway, thank you so much for this enhancement!
Comment 11 Jack D. Pond 2010-02-12 13:15:15 UTC
Any idea where this one stands?
Comment 12 Jack D. Pond 2010-05-07 15:09:55 UTC
Created attachment 7355 [details]
introtemplate and outrotemplate functionality + params passing fix to Special:Ask - patched to current HEAD

Modified patch to apply to current head (as of r66007, last update to SMW_QP_List.php in r65468)
Comment 13 Jack D. Pond 2010-06-29 14:34:10 UTC
Created attachment 7526 [details]
Advanced Formatting and "further results" fix

Current HEAD Patch

Advanced formatting
fixes "further results" and;
compensates for more recent changes in SMW (specifically SMW_QueryPrinter.php)
Comment 14 Jack D. Pond 2010-06-29 19:13:23 UTC
Created attachment 7527 [details]
Advanced Formatting and "further results" fix - format/version fixes

Fixed formatting and added messages for thoroughness.  Thanks to help from Yaron
Comment 15 Jack D. Pond 2010-07-06 22:26:49 UTC
Fixed and tested: r68747
Comment 16 Sal Quintanilla 2012-09-22 02:49:56 UTC
I have some intro templates that take arguments, so they end up looking like
<pre>
|intro={{displayQueryTableHeader |titleHeader=Ballparks |dataHeader=State |sort=off }}
</pre>
in my inline query.

I've had to limit tables lately and saw the problem, which is what brought me here.  introtemplate / outrotemplate work great, but it looks I'll need discrete intro templates for every table variation I use.  I tried the obvious in SMW_QP_List.php, namely
<pre>
-          $result .= "{{" . $this->mIntroTemplate . "}}";
+          $result .= $this->mIntroTemplate;
</pre>
and that almost works, except that my table headers in further results look like
<pre>
Project UNIQ4c0b9a8d5d40c6a0-nowiki-00000002-QINU DateUNIQ4c0b9a8d5d40c6a0- nowiki-00000003-QINU 	BallparkUNIQ4c0b9a8d5d40c6a0-nowiki-00000004-QINU
</pre>
instead of
<pre>
Project  Date  Ballpark
</pre>
I'll keep looking for ways around this, but if it's obvious to someone here please let us know.

Thanks.
Comment 17 Sal Quintanilla 2012-10-01 15:31:15 UTC
Re-opened for additional consideration regarding my comment on 9/22 above.
Comment 18 Jack D. Pond 2012-10-01 16:11:48 UTC
(In reply to comment #16)

Sal,

You are correct - and it is a complete PITA requiring custom templates.  However, the way we dealt with it is that we needed the custom template for the table headers anyway and we used the same template for both (the table and the search) as an inclusion.

I haven't worked on Semantic MediaWiki in a while and I'm not currently doing any development with MediaWiki other than some conversion and support.

If you have any ideas on how to avoid this unfortunate requirement (custom header/footer templates), I'm all ears.
Comment 19 Sal Quintanilla 2012-10-01 16:26:21 UTC
Hi Jack,

Don't get me wrong, I don't want to avoid them.  There are cases where they're needed, and this interface fine for it.  

The only issue I'm seeing is that introtemplate and outrotemplate aren't really templates in the context of mediawiki.  They're more like introtext and outrotext, since they can't have arguments passed in.  I'd like to see them work like templates that allow me to pass in arguments.
Comment 20 Jack D. Pond 2012-10-01 17:06:54 UTC
(In reply to comment #19)
Sal,

Again, you are right. When I wrote the patch, it was to use the existing query function, that required the use of an incomplete header (no closing }}) and an incomplete footer (no opening {{) without changing the SemanticMediaWiki code, only adding what basically is a hook.  I should not have used the word "Template" in there - it is misleading.

It overcame a deficiency without causing other issues - but it is NOT the right answer.

The correct answer would be to include the capability you mention - but it would also require more extensive modification of code.

If you can get buy-in from Markus and/or Yaron, I'll look into it.  Otherwise, I've done some work in the past that went to naught because it was not considered in keeping with the "intellectual integrity" of SMW, even though it was essential to meet my users' needs and I'm hesitant to make the investment.
Comment 21 Sal Quintanilla 2012-10-01 22:46:49 UTC
Good-ish news, on further examination, I'm causing the error.  The simple patch I described above of removing the '{{' and '}}' from the introtemplate and outrotemplate assignment lines works as-is, allowing arguments to the template.

But my template not only took arguments, but also tried to make a table with a variable number of columns.  It did so by using an introtemplate like this:

{| 
! style="background:#efefef;" | {{{col1|}}} {{#ifeq: {{#expr:{{{colCount}}} > 1}} | 1 | <div style="display:none">{{</div><nowiki />
! style="background:#efefef;" | {{{col2|}}}<div style="display:none">}}</div>}}{{#ifeq: {{#expr:{{{colCount}}} > 2}} | 1 | <div style="display:none">{{</div><nowiki />
! style="background:#efefef;" | {{{col3|}}}<div style="display:none">}}</div> }}
|-

This looks as intended in the original table, but the further results link gets formed with 

http://localhost/index.php?title=Special:Ask...&introtemplate=%0A%7B%7C%20%20border%3D%221%22%20cellpadding%3D%225%22%20cellspacing%3D%220%22%20class%3D%22wikitable%20sortable%22%0A%21%20style%3D%22background%3A%23efefef%3B%22%20%20%7C%20Project%20%3Cdiv%20id%3D%22col2start%22%20style%3D%22display%3Anone%22%3E%7B%7B%3C%2Fdiv%3E%7FUNIQ159a63d57ad7d99f-nowiki-00000000-QINU%7F%0A%21%20style%3D%22background%3A%23efefef%3B%22%20%20%7C%20Date%3Cdiv%20id%3D%22col2end%22%20style%3D%22display%3Anone%22%3E%7D%7D%3C%2Fdiv%3E%3Cdiv%20id%3D%22col3start%22%20style%3D%22display%3Anone%22%3E%7B%7B%3C%2Fdiv%3E%7FUNIQ159a63d57ad7d99f-nowiki-00000001-QINU%7F%0A%21%20style%3D%22background%3A%23efefef%3B%22%20%20%7C%20Ballpark%3Cdiv%20id%3D%22col3end%22%20style%3D%22display%3Anone%22%3E%7D%7D%3C%2Fdiv%3E%3Cdiv%20%22col4start%22%20style%3D%22display%3Anone%22%3E%7B%7B%3C%2Fdiv%3E%7FUNIQ159a63d57ad7d99f-nowiki-00000002-QINU%7F%0A%21%20style%3D%22background%3A%23efefef%3B%22%20%20%7C%20Estimator%3Cdiv%20id%3D%22col4end%22%20style%3D%22display%3Anone%22%3E%7D%7D%3C%2Fdiv%3E%3Cdiv%20style%3D%22display%3Anone%22%3E%7B%7B%3C%2Fdiv%3E%7FUNIQ159a63d57ad7d99f-nowiki-00000003-QINU%7F%0A...

which does not get parsed like it's supposed and causes the UNIQ...QINU labels to display.  

I'm going to ask on the list if anyone has gotten if-tests to work with further results, but as far as this goes, the patch below at least gives introtemplate / outrotemplate argument support, iff this syntax is used:

|introtemplate={{intemplate |arg1=arg1 |arg2=arg2}}
|outrotemplate={{outtemplate}}

Index: SMW_QP_List.php
===================================================================
--- SMW_QP_List.php     (revision 259)
+++ SMW_QP_List.php     (working copy)
@@ -98,7 +98,7 @@
                        $rows_in_cur_column = 0;
                }
                if ( $this->mIntroTemplate != '' ) {
-                       $result .= "{{" . $this->mIntroTemplate . "}}";
+                       $result .= $this->mIntroTemplate;
                }

                // Now print each row
@@ -169,7 +169,7 @@
                        $result .= $rowend;
                }
                if ( $this->mOutroTemplate != '' ) {
-                       $result .= "{{" . $this->mOutroTemplate . "}}";
+                       $result .= $this->mOutroTemplate;
                }

                // Make label for finding further results
Comment 22 Andre Klapper 2012-11-05 14:51:19 UTC
(In reply to comment #21)
> I'm going to ask on the list if anyone has gotten if-tests to work with further
> results, but as far as this goes, the patch below at least gives introtemplate
> / outrotemplate argument support, iff this syntax is used:

Sal: Did you ask? What were the answers?
Comment 24 Jeroen De Dauw 2013-04-02 02:10:55 UTC
Removing as blocker for 1.9
Comment 25 Sal Quintanilla 2013-04-12 20:59:37 UTC
Update for Wikimedia Bug Day - This is still a problem, as described above.
Comment 26 MWJames 2013-06-10 03:24:41 UTC
(In reply to comment #25)
> Update for Wikimedia Bug Day - This is still a problem, as described above.

Necessary changes should be available in Gerrit to ensure an appropriate review process.

If changes consists of new or changed behaviour then changes should be accompanied by unit tests to verify that added/changed functionality did not alter existing features and keep possible regressions at a minimum.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links