Good afternoon,

I've noted that when you use the Remove Trailing Zero Argument option, the trailing zero will be removed (it's ok, that's the idea), but it will remove all the zeros before the last "/".

For example, if you have the url http://www.example.com/taxonomy/term/10/0, you'll be redirected to http://www.example.com/taxonomy/term/1 (as you can see, the taxonomy term id changed from 10 to 1). The same thing will happen if your term id is 100, 1000, 10000, etc, all of them will redirect to the term id 1.

I'm available if you need me to do some tests.

Good bye.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YK85’s picture

subscribing

nicholasThompson’s picture

Version: 6.x-1.2 » 6.x-1.x-dev
Status: Active » Needs work

Well spotted! I can confirm this as a bug and will get this sorted in the next release.

mikeytown2’s picture

+1

juankvillegas’s picture

Great news... thank you for taking care of this.

I'll try to modify my Global Redirect version until yours is available.

jiakomo’s picture

Hello,
is this fixed in current -dev ?

YK85’s picture

I was wondering if any further progress has been made with this issue?
Thank you very much

juankvillegas’s picture

Status: Needs review » Needs work

I modified it manually because the current 6.x-1.x-dev version hasn't solved it yet... and the solution is very very very very simple.

Between lines 112 and 116 it has:

    case 1 :
      // If last 2 characters of URL are /0 then trim them off
      if (drupal_substr($request, -2) == '/0') {
        $request = rtrim($request, '/0');
      }

Just replace line 115 to have this:

    case 1 :
      // If last 2 characters of URL are /0 then trim them off
      if (drupal_substr($request, -2) == '/0') {
        $request = drupal_substr($request, 0, -2); // THIS IS THE LINE REPLACED.
      }
mikeytown2’s picture

Status: Needs work » Needs review
FileSize
437 bytes
jiakomo’s picture

Status: Needs work » Needs review

This seems to work for me with no side-effects.

mikeytown2’s picture

Status: Needs review » Reviewed & tested by the community
juankvillegas’s picture

There is a new dev version and this was not included. What can we do to have this included?

Dave Reid’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Priority: Critical » Major
Status: Reviewed & tested by the community » Needs review
FileSize
1.17 KB

Last patch did not include any test to verify that it works nor prevent this functionality from regressing again. Attached patch includes a test that should help round this out.

Dave Reid’s picture

Status: Needs review » Needs work

Hrm, seems the tests don't actually check with the de-trailing zero option enabled at all.

nicholasThompson’s picture

Status: Needs work » Fixed

Fix from #8 (D6) and #12 (D7) committed to dev branch.

Also updated the test suites on both to include the taxonomy/term/10/0 test AND fixed the missing trailing_zero settings toggle (well spotted Dave!).

In doing so its exposed an interesting behaviour in D7 where we expect it to 301 on the 10/0 term if trailing_zero is enabled, however if you have menu_check enabled too it will just 404 as the term doesn't exist (and therefore the menu item fails?! I guess...) Due to this I have separated out the menu_check test and trailing_zero test.

Maybe in the future, we should improve the test suite to handle more setting combinations.

Status: Fixed » Closed (fixed)

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