When using the jump menu with a grouping field, the contents of this field is escaped using the url() function, transforming any non-ascii characters into url-encoded form. This can be fixed by commenting the line 91 in views_plugin_style_jump_menu.inc :

$grouping = url($grouping);

Is this line even necessary, it's just a label and not any form of link.

CommentFileSizeAuthor
#26 topic_subtopic_jump.jpg68.35 KBlhugg
#22 746846-fix-slightly-broken-jump-menu2.patch3.06 KBmerlinofchaos
#21 746846-fix-slightly-broken-jump-menu.patch2.44 KBmerlinofchaos
#16 746846-proposed_fix2.patch894 byteslavamind
#6 746846-screenshot.png10.74 KBlavamind
#6 746846-proposed_fix.patch826 byteslavamind
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Can you provide a patch?

I'm not sure whether this really fixes the Problem, because the goal of the jumping menu has to be a url.

lavamind’s picture

lavamind CreditAttribution: lavamind commented

Here's the patch :

--- a/plugins/views_plugin_style_jump_menu.inc	2010-03-19 00:02:40.000000000 -0400
+++ b/plugins/views_plugin_style_jump_menu.inc	2010-03-19 11:23:40.877613283 -0400
@@ -88,7 +88,6 @@
           if ($this->view->field[$this->options['grouping']]->options['label']) {
             $grouping = $this->view->field[$this->options['grouping']]->options['label'] . ': ' . $grouping;
           }
-          $grouping = url($grouping);
         }
         $sets[$grouping][] = $row;
       }

The jump menu options should indeed have a URL as value, but this function doesn't seem related to that specific line. The $grouping value goes into the "label" parameter of the "optgroup" tag, which is defined as a text field according to the HTML spec : http://www.w3.org/TR/html401/interact/forms.html#adef-label-OPTGROUP

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

A patch file would be better.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs work

.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

url() ensured that it was fully qualified, which is necessary if your base_url is something like http://www.example.com/foo or if you are not using clean URLs. It is necessary.

lavamind’s picture

lavamind CreditAttribution: lavamind commented
FileSize
746846-proposed_fix.patch826 bytes
746846-screenshot.png10.74 KB

I've attached a screenshot of the problem, to make the things clearer. As you can see it's the grouping label, and only the grouping label, that appears URL-encoded. The corrupted value displayed are role titles, and they appear correctly everywhere else on the site. Looking at the markup, the select option values (URL paths, usually) do not appear to be processed by url() at all.

I've looked at the problem more closely and it appears that it's simply the wrong value that's passed through url(). It should be $path instead of $grouping, the latter being nothing more than a text label. I've checked in 6.x-2.x-dev and the problem is the same there.

Here's a patch file to apply against 6.x-2.x-dev.

I verified that 1) it fixes the group encoding problem and 2) it applies the url() filter to the proper value.

If this patch is accepted it might be useful to note that with this change, in some cases it might be necessary to review the settings on views fields exposed as the jump menu URL path : in my case I had added a forward-slash at the beginning of the field value rewrite (ie. /user/[uid] instead of user/[uid]), which caused confusion in url().

khosman’s picture

khosman CreditAttribution: khosman commented

Thanks, lavamind -

I applied your changes and solved the issue with the group listing - but a problem remains with the listings appearing under the groups. Specifically, if they have a single quote/apostrophe, it appears encoded. Do you have a suggestion as to how to fix this?

ñull’s picture

ñull CreditAttribution: ñull commented
Version: 6.x-2.8 » 6.x-2.10

You applied it but I still see the issue in my version. Could you redo it that it gets included for version 6.x-2.11 ?

ñull’s picture

ñull CreditAttribution: ñull commented

I worked around it by using custom PHP field for the fields I wanted included there:

  echo $data->node_title." by ".$data->users_name;

However in your code you could probably use something like html_entity_decode

khosman’s picture

khosman CreditAttribution: khosman commented

Wow - well, that workaround works great...but... how and why? I put the variable I wanted to see used as the choices in the dropdown into your code and it worked. But why does appending/concatenating another field (users_name) cause it to read the apostrophes correctly?

HunterElliott’s picture

HunterElliott CreditAttribution: HunterElliott commented

Hmm... well, for me, it's not a path/grouping issue (or so I think), because I have no grouping going on in my dropdown/jump menu and the "jumps" do work. However, all of my text that has ampersands and/or apostrophes is being parsed out into entities. For instance, George's appears as Georges' or Restaurant & Wine bar appears as Restaurant & Wine Bar

I tried the patch above and my problem still exists. Suggestions? I saw someone having a similar issue but it was marked as a duplicate of this thread.

khosman’s picture

khosman CreditAttribution: khosman commented

HunterElliot, scroll up to comment #9 from "com2." His first fix worked for me (per my comment) - but I ended up going with his second suggestion, namely to use "html_entity_decode." Follow his link above (again, many thanks, com2) for instructions - its simple and works beautifully.

Here, specifically, is the code I employed:

echo html_entity_decode($data->node_title, ENT_QUOTES);

Toss in the opening and closing php tags and it works like a charm.

HunterElliott’s picture

HunterElliott CreditAttribution: HunterElliott commented

well, I went to try and theme my jump menu view, and it's coming back with the error:
"Style output: views-view-jump-menu.tpl.php (File not found, in folder sites/all/modules/views/theme/)"

Could this be part of the issue? I'm looking back through my previous versions of this module and am not finding this file in any of them so far.

Is this file needed?

CubicX’s picture

CubicX CreditAttribution: CubicX commented

Indeed, this file seems to be missing in all theme's.

HunterElliott’s picture

HunterElliott CreditAttribution: HunterElliott commented

Thanks, khosman (sorry for the delay in a follow-up) - I basically did this.

The actual jump menu I created worked fine, just the text output was "funky." I created a .tpl.php file specifically for my Title field (of course, you could do it for any field in the View, I just used the template off of the themeinfo: views-view-field.tpl.php).

All I did in it was put the following:

<?php 
$a = $output;
$b = html_entity_decode($a, ENT_QUOTES);
print $b; 
 ?>

and that cleared everything up.

lavamind’s picture

lavamind CreditAttribution: lavamind commented
FileSize
746846-proposed_fix2.patch894 bytes

@khosman, HunterElliot: Your issues with html entities could be fixed by replacing $grouping = url($grouping); with $grouping = html_entity_decode($grouping);, instead of deleting the line completely as my previous proposed patch does, see #6.

However we should be careful about quotes because that string is placed into a HTML parameter that's surrounded by double-quotes. If html_entity_decode produces such quotes in 'normal' form, the label will be cut-off and the resulting HTML will be invalid. A safer solution would be to add the ENT_NOQUOTES parameter, though single quotes still won't be converted (that could be done seperately).

EDIT: I re-read the thread and it's not clear if your issues are related to the grouping label and/or the option list items only. If your issue is with option list items only (which I think it is), then this new patch doesn't apply, please disregard it. Furthermore, issues with option list items in the jump menu should probably be treated in a seperate issue thread. This issue was opened only to deal with grouping labels and that's what my first patch (#6) deals with.

lavamind’s picture

lavamind CreditAttribution: lavamind commented
Status: Needs work » Reviewed & tested by the community

Despite unrelated encoding problems raised in this thread, I think the patch in #6 properly deals with the discrete problem I raised in my initial message.

lliss’s picture

lliss CreditAttribution: lliss commented

Patch discussed in #6 is pretty clean and fixed the problem. Nice work. Thanks.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Applied the patch in #6 to all branches.

Gerhard Killesreiter’s picture

Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented
Status: Fixed » Active

#6 broke a jump menu view of mine.

I have set it up without a grouping field but using Nid as a path field. The nid field is set to be excluded and rewritten using node/[nid]/edit.

After applying the patch the redirect would happen to http://node/n/edit, ie the domain was missing.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Active » Needs review
FileSize
746846-fix-slightly-broken-jump-menu.patch2.44 KB

I believe this fixes killes' problem.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
746846-fix-slightly-broken-jump-menu2.patch3.06 KB

Hm. Another try, it seems killes is doing query string tricks too, and those used to work but no longer.

Gerhard Killesreiter’s picture

Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented
Status: Needs review » Reviewed & tested by the community

Yep, this works!

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Reviewed & tested by the community » Fixed

Committed to all branches.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

lhugg’s picture

lhugg CreditAttribution: lhugg commented
Status: Closed (fixed) » Active
FileSize
topic_subtopic_jump.jpg68.35 KB

It doesn't look like this patch made it to 6.12. I'm having the same issues. Shouldn't these changes have remained in the branch?

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Active » Closed (fixed)

Please read 2.12's release notes.

lhugg’s picture

lhugg CreditAttribution: lhugg commented

So what does "committed to all branches" mean? Only code obtained through CVS? Should we apply the patch to the downloaded or Acquia versions?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Commited to all branches means it's commited to the 2.x-dev version.

But as you can read on the release announcement 2.12 is exactly 2.11 with one patch: the security patch.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

By the way, 'Please read the release notes' does not mean 'ignore the release notes and ask anyway,' it means read the release notes. If you had read the release notes, like I said, you would not have needed to ask the followup question.

lhugg’s picture

lhugg CreditAttribution: lhugg commented

Thanks dereine for answering my question, which is what is meant in drupalese by "Committed to all branches." I did read the notes and understood that this patch was not in release 2.12, but still wanted to know for future reference what a developer meant specifically when they said that a patch had been applied to all branches. Apparently that means only the dev versions.

Thank you for taking the time to help me out. The more learn, the stronger we become as a community.