mark: A photo of Mark kneeling on top of the Taal Volcano in the Philippines. It was a long hike. (Default)
Mark Smith ([staff profile] mark) wrote in [site community profile] changelog2010-01-06 12:29 am
  • Add Memory
  • Share This Entry

[dw-free] Logic for checking format=light/style=mine, etc arguments is duplicated in various places

[commit: http://hg.dwscoalition.org/dw-free/rev/75f98a293c1d]

http://bugs.dwscoalition.org/show_bug.cgi?id=1968

More work to abstract out some arguments we use in dozens of places. Fix
persistence issue causing some permutations not to persist right.

Patch by [personal profile] afuna.

Files modified:
  • cgi-bin/LJ/S2.pm
  • cgi-bin/LJ/S2/EntryPage.pm
  • cgi-bin/LJ/S2/ReplyPage.pm
  • cgi-bin/LJ/Talk.pm
  • cgi-bin/weblib.pl
  • htdocs/talkpost.bml
  • htdocs/talkpost_do.bml
  • htdocs/talkread.bml
--------------------------------------------------------------------------------
diff -r 4e1c7180dbc0 -r 75f98a293c1d cgi-bin/LJ/S2.pm
--- a/cgi-bin/LJ/S2.pm	Wed Jan 06 00:24:59 2010 +0000
+++ b/cgi-bin/LJ/S2.pm	Wed Jan 06 00:29:51 2010 +0000
@@ -2084,6 +2084,7 @@ sub Page
         'head_content' => '',
         'data_link' => {},
         'data_links_order' => [],
+        '_styleopts' => \%{ LJ::viewing_style_opts( %$get ) },
     };
 
     if ($LJ::UNICODE && $opts && $opts->{'saycharset'}) {
@@ -3278,7 +3279,7 @@ sub _print_reply_container
 
         my $userpic = LJ::ehtml($page->{'_picture_keyword'}) || "";
         my $thread = $page->{'viewing_thread'} + 0 || "";
-        $S2::pout->(LJ::create_qr_div($u, $ditemid, $page->{'_stylemine'} || 0, $userpic, $thread));
+        $S2::pout->( LJ::create_qr_div( $u, $ditemid, $page->{_styleopts}, $userpic, $thread ) );
     }
 }
 
diff -r 4e1c7180dbc0 -r 75f98a293c1d cgi-bin/LJ/S2/EntryPage.pm
--- a/cgi-bin/LJ/S2/EntryPage.pm	Wed Jan 06 00:24:59 2010 +0000
+++ b/cgi-bin/LJ/S2/EntryPage.pm	Wed Jan 06 00:29:51 2010 +0000
@@ -371,7 +371,6 @@ sub EntryPage
                     js/commentmanage.js
                     ));
 
-    $p->{'_stylemine'} = $get->{'style'} eq 'mine' ? 1 : 0;
     $p->{'_picture_keyword'} = $get->{'prop_picture_keyword'};
 
     $p->{'viewing_thread'} = $get->{'thread'} ? 1 : 0;
diff -r 4e1c7180dbc0 -r 75f98a293c1d cgi-bin/LJ/S2/ReplyPage.pm
--- a/cgi-bin/LJ/S2/ReplyPage.pm	Wed Jan 06 00:24:59 2010 +0000
+++ b/cgi-bin/LJ/S2/ReplyPage.pm	Wed Jan 06 00:29:51 2010 +0000
@@ -229,8 +229,8 @@ sub ReplyPage
         '_u' => $u,
         '_ditemid' => $ditemid,
         '_parpost' => $parpost,
-        '_stylemine' => $get->{'style'} eq "mine",
         '_values' => \%comment_values,
+        '_styleopts' => $p->{_styleopts},
     };
 
     return $p;
@@ -254,7 +254,7 @@ sub ReplyForm__print
                                      'parpost'   => $parpost,
                                      'replyto'   => $parent,
                                      'ditemid'   => $form->{'_ditemid'},
-                                     'stylemine' => $form->{'_stylemine'},
+                                     'styleopts' => $form->{_styleopts},
                                      'form'      => $post_vars, 
                                      'do_captcha' => LJ::Talk::Post::require_captcha_test($remote, $u, $post_vars->{body}, $form->{'_ditemid'})}));
 
diff -r 4e1c7180dbc0 -r 75f98a293c1d cgi-bin/LJ/Talk.pm
--- a/cgi-bin/LJ/Talk.pm	Wed Jan 06 00:24:59 2010 +0000
+++ b/cgi-bin/LJ/Talk.pm	Wed Jan 06 00:29:51 2010 +0000
@@ -1170,7 +1170,6 @@ sub talkform {
     # parpost:     parent post object
     # replyto:     init->replyto
     # ditemid:     init->ditemid
-    # stylemine:   user using style=mine or not
     # form:        optional full form hashref
     # do_captcha:  optional toggle for creating a captcha challenge
     # require_tos: optional toggle to include TOS requirement form
@@ -1220,13 +1219,16 @@ sub talkform {
         $ret .= "<hr />";
     }
 
+    $opts->{styleopts} ||= LJ::viewing_style_opts( %$form );
+
     # hidden values
     my $parent = $opts->{replyto}+0;
     $ret .= LJ::html_hidden("replyto", $opts->{replyto},
                             "parenttalkid", $parent,
                             "itemid", $opts->{ditemid},
                             "journal", $journalu->{'user'},
-                            "stylemine", $opts->{stylemine});
+                            %{$opts->{styleopts}},
+                            );
 
     # rate limiting challenge
     {
diff -r 4e1c7180dbc0 -r 75f98a293c1d cgi-bin/weblib.pl
--- a/cgi-bin/weblib.pl	Wed Jan 06 00:24:59 2010 +0000
+++ b/cgi-bin/weblib.pl	Wed Jan 06 00:29:51 2010 +0000
@@ -659,20 +659,21 @@ sub check_form_auth {
 # class: web
 # des: Creates the hidden div that stores the QuickReply form.
 # returns: undef upon failure or HTML for the div upon success
-# args: user, remote, ditemid, stylemine, userpic
+# args: user, remote, ditemid, style args, userpic, viewing thread
 # des-u: user object or userid for journal reply in.
 # des-ditemid: ditemid for this comment.
-# des-stylemine: if the user has specified style=mine for this page.
+# des-style_opts: the viewing style arguments on this page, as a hashref.
 # des-userpic: alternate default userpic.
 # </LJFUNC>
 sub create_qr_div {
 
-    my ($user, $ditemid, $stylemine, $userpic, $viewing_thread) = @_;
+    my ( $user, $ditemid, $style_opts, $userpic, $viewing_thread ) = @_;
     my $u = LJ::want_user($user);
     my $remote = LJ::get_remote();
     return undef unless $u && $remote && $ditemid;
 
-    $stylemine ||= 0;
+    $style_opts ||= {};
+
     my $qrhtml;
 
     LJ::load_user_props($remote, "opt_no_quickreply");
@@ -681,7 +682,7 @@ sub create_qr_div {
     $qrhtml .= "<div id='qrformdiv'><form id='qrform' name='qrform' method='POST' action='$LJ::SITEROOT/talkpost_do'>";
     $qrhtml .= LJ::form_auth();
 
-    my $stylemineuri = $stylemine ? "style=mine&" : "";
+    my $stylemineuri = $style_opts ? LJ::viewing_style_args( $style_opts ) : "";
     my $basepath =  LJ::journal_base($u) . "/$ditemid.html?${stylemineuri}";
     my $usertype = ($remote->openid_identity && $remote->is_validated) ? 'openid_cookie' : 'cookieuser';
     $qrhtml .= LJ::html_hidden({'name' => 'replyto', 'id' => 'replyto', 'value' => ''},
@@ -693,9 +694,11 @@ sub create_qr_div {
                                {'name' => 'cookieuser', 'id' => 'cookieuser', 'value' => $remote->{'user'}},
                                {'name' => 'dtid', 'id' => 'dtid', 'value' => ''},
                                {'name' => 'basepath', 'id' => 'basepath', 'value' => $basepath},
-                               {'name' => 'stylemine', 'id' => 'stylemine', 'value' => $stylemine},
                                {'name' => 'viewing_thread', 'id' => 'viewing_thread', 'value' => $viewing_thread},
                                );
+    while ( my ( $key, $value ) = each %$style_opts ) {
+        $qrhtml .= LJ::html_hidden( { name => $key, id => $key, value => $value } );
+    }
 
     # rate limiting challenge
     {
@@ -1013,19 +1016,39 @@ and returns them as a string that can be
 =cut
 sub viewing_style_args {
     my ( %args ) = @_;
+
+    %args = %{ LJ::viewing_style_opts( %args ) };
+
+    my @valid_args;
+    while ( my ( $key, $value ) = each %args ) {
+            push @valid_args, "$key=$value";
+    }
+
+    return join "&", @valid_args;
+}
+
+=head2 C<< LJ::viewing_style_opts( %arguments ) >>
+Takes a list of viewing styles arguments from a list, and returns a hashref of valid values
+=cut
+
+sub viewing_style_opts {
+    my ( %args ) = @_;
+
     my $valid_style_args = {
         format => { light => 1 },
         style  => { light => 1, site => 1, mine => 1 },
     };
-    
-    my @valid_args;
+
+    my %ret;
+
     # only accept purely numerical s2ids
-    push @valid_args, "s2id=$args{s2id}" if $args{s2id} && $args{s2id} =~ /^\d+$/;
+    $ret{s2id} = $args{s2id} if $args{s2id} && $args{s2id} =~ /^\d+$/;
 
-    foreach my $key ( qw( format style ) ) {
-         push @valid_args, "style=$args{$key}" if $valid_style_args->{$key}->{$args{$key}};
+    for my $key( qw ( format style ) ) {
+        $ret{$key} = $args{$key} if $valid_style_args->{$key}->{$args{$key}};
     }
-    return join "&", @valid_args;
+
+    return \%ret;
 }
 
 # <LJFUNC>
diff -r 4e1c7180dbc0 -r 75f98a293c1d htdocs/talkpost.bml
--- a/htdocs/talkpost.bml	Wed Jan 06 00:24:59 2010 +0000
+++ b/htdocs/talkpost.bml	Wed Jan 06 00:29:51 2010 +0000
@@ -111,9 +111,6 @@ body<=
     }
 
     my $itemid = $init->{'itemid'};
-
-    my $formatlight = ( ( $GET{format} eq 'light' ) || ( $GET{style} eq 'light' ) ) ? 'style=light' : '';
-    my $stylemine = $init->{style} eq "mine" ? "style=mine" : "";
 
     ## load the journal item
     my $item = LJ::Talk::get_journal_item($u, $itemid);
@@ -394,7 +391,6 @@ body<=
                                  'parpost'   => $parpost,
                                  'replyto'   => $init->{replyto},
                                  'ditemid'   => $ditemid,
-                                 'stylemine' => $GET{'style'} eq "mine",
                                  'form'      => \%FORM,
                                  'do_captcha' => LJ::Talk::Post::require_captcha_test($remote, $u, $FORM{body}, $ditemid)});
 
diff -r 4e1c7180dbc0 -r 75f98a293c1d htdocs/talkpost_do.bml
--- a/htdocs/talkpost_do.bml	Wed Jan 06 00:24:59 2010 +0000
+++ b/htdocs/talkpost_do.bml	Wed Jan 06 00:29:51 2010 +0000
@@ -157,7 +157,6 @@ body<=
                                     'ditemid'     => $POST{itemid},
                                     'require_tos' => $require_tos,
                                     'do_captcha'  => $need_captcha,
-                                    'stylemine'   => $GET{'style'} eq "mine",
                                     'errors'      => \@errors,
                                     'form'        => \%POST });
     }
@@ -218,7 +217,7 @@ body<=
     # Yeah, we're done.
     my $dtalkid = $comment->{talkid}*256 + $item->{anum};
 
-    # Allow style=mine for QR redirects
+    # Allow style=mine, etc for QR redirects
     my $style_args = LJ::viewing_style_args( %POST );
 
     my $cthread = $POST{'viewing_thread'} ? "thread=$POST{viewing_thread}" : "view=$dtalkid";
diff -r 4e1c7180dbc0 -r 75f98a293c1d htdocs/talkread.bml
--- a/htdocs/talkread.bml	Wed Jan 06 00:24:59 2010 +0000
+++ b/htdocs/talkread.bml	Wed Jan 06 00:29:51 2010 +0000
@@ -809,7 +809,7 @@ body<=
 
         $ret .= "<div align='center'>" . LJ::make_qr_target('top') . "</div>" if $remote;
 
-        my $stylemine = $GET{'style'} eq 'mine' ? 1 : 0;
+        my $styleopts = LJ::viewing_style_opts( %GET );
 
         #
         # Comment thread display
@@ -818,7 +818,7 @@ body<=
         if (defined $GET{'thread'}) {
             $viewing_thread = $GET{'thread'};
         }
-        $ret .= LJ::create_qr_div($u, $ditemid, $stylemine, $GET{'prop_picture_keyword'}, $viewing_thread);
+        $ret .= LJ::create_qr_div( $u, $ditemid, $styleopts, $GET{prop_picture_keyword}, $viewing_thread );
 
         $ret .= LJ::html_hidden("ditemid", $ditemid);
         $ret .= LJ::html_hidden("journal", $u->{'user'});
--------------------------------------------------------------------------------