645427 - Bugzilla->usage_mode( USAGE_MODE_BROWSER ) doesn't change Bugzilla->usage_mode

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Yavor Nikolov]

User-Agent:       Mozilla/5.0 (Windows NT 6.0; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: 4.1.1

Bugzilla->usage_mode( Bugzilla::Constants->USAGE_MODE_BROWSER ) doesn't change Bugzilla->usage_mode to USAGE_MODE_BROWSER

Reproducible: Always

Steps to Reproduce:
print( "USAGE_MODE_BROWSER: ", Bugzilla::Constants->USAGE_MODE_BROWSER, "\n" );
print( "USAGE_MODE_CMDLINE: ", Bugzilla::Constants->USAGE_MODE_CMDLINE, "\n" );

print( "usage_mode before change: ", Bugzilla->usage_mode, "\n" );
my $res1 = Bugzilla->usage_mode( Bugzilla::Constants->USAGE_MODE_BROWSER );
print( "usage_mode during change: ", $res1, "\n" );
print( "usage_mode after change: ", Bugzilla->usage_mode, "\n" );

Actual Results:  
USAGE_MODE_BROWSER: 0
USAGE_MODE_CMDLINE: 1
usage_mode before change: 1
usage_mode during change: 1
usage_mode after change: 1


Expected Results:  
USAGE_MODE_BROWSER: 0
USAGE_MODE_CMDLINE: 1
usage_mode before change: 1
usage_mode during change: 0
usage_mode after change: 0


Root cause is return statement in Bugzilla::usage_mode:

    return $class->request_cache->{usage_mode}
        || (i_am_cgi()? USAGE_MODE_BROWSER : USAGE_MODE_CMDLINE);

When $class->request_cache->{usage_mode} == 0 first part of the || expression is always discarded.

Comment 1

[Yavor Nikolov]

Looking at usage_mode code of stable older releases - issues seems to exist since 3.2

Comment 2

[Yavor Nikolov]

Sorry... I meant since 3.4 (not 3.2)

Comment 3

[Frédéric Buclin]

We should probably check "exists $class->request_cache->{usage_mode}".

Comment 4

[Max Kanat-Alexander]

Oh, exists is a good idea, yeah.

Comment 5

[Koosha KM]

Attachment: V1

Comment 6

[Frédéric Buclin]

Comment on attachment 617193 [details] [diff] [review]
V1

>+    if (exists($class->request_cache->{usage_mode})) {
>+        return $class->request_cache->{usage_mode};
>+    }
>+    else {
>+        return (i_am_cgi()? USAGE_MODE_BROWSER : USAGE_MODE_CMDLINE);
>+    }

Once we require Perl 5.10.1, all this code can simply be replaced by:

    return $class->request_cache->{usage_mode}
      // (i_am_cgi()? USAGE_MODE_BROWSER : USAGE_MODE_CMDLINE);

I will add a comment above this code so that we remember to do it in Bugzilla 4.6. r=LpSolit

Comment 7

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla.pm
Committed revision 8213.