[python-committers] Code review tool (rietveld) bug

[Yury Selivanov]

Martin,

On 1/27/2014, 5:01 PM, "Martin v. Löwis" wrote:
> Am 27.01.14 21:18, schrieb Yury Selivanov:
>> OK, I've tried another browser (regularly I use Safari, this
>> time I was trying it with Chrome) -- same thing.
>>
>> For those who want to try to reproduce it:
>>
>> 1. Open http://bugs.python.org/issue20356
>> 2. Click 'review' for pos_only_format_02.patch
>> 3. Click on message from @larry
>> 4. Hit 'reply' link for it
>> 5. Erase everything from the text area and type something in
>> 5. Submit the form.
> I can add a message just fine, but I think something is broken still.
>
> Can you get your web browser to see the source of the reply form? I get
>
>      <form method="POST" action="/review/20356/publish"
>            id="message-reply-form">
>        <input type="hidden" name="xsrf_token"
> value="7e667aa15c51043fea022aa837edaaa5">
>        <div></div>
>        <input type="hidden" name="in_reply_to" value="" />
>        <input type="hidden" name="subject" value="fix formatting of
> positional-only parameters in inspect.Signature" />
>        <input type="hidden" name="message_only" value="1" />
>        <input type="submit" value="Send Message" />
>        <input type="button" value="Discard" name="discard" />
>        <input type="checkbox" name="send_mail" value="1"
>               id="message-reply-send-mail" checked="checked" />
>        <label>Send mail to reviewers</label>
>      </form>
>
> I believe the issue is the hidden in_reply_to field.
>
> a) it shouldn't be an empty string (I think); if it wasn't, Rietveld
>     might actually make insert it in a threaded way. IIUC, js ought
>     to have inserted a value for in_reply_to.
> b) if it is empty, it apparently crashes for you because it then finds
>     that there is no message with the id "".
>
> Looking a bit further - maybe Safari doesn't tell me the dynamic code.
> I also see
>
> <a href="javascript:M_replyToMessage('0', '2014/01/25 11:25:44',
> 'larry', 'Message_2687')"
>                   id="message-reply-href-0">Reply</a>
>
> which really ought to fill out the in_reply_to field (with Message_2687)

Sure, here is the form markup:


<form method="POST" action="/review/20356/publish" 
id="message-reply-form"><textarea rows="7" cols="70" name="message" 
style=""></textarea>
       <input type="hidden" name="xsrf_token" 
value="769e2ad622940b39cde28df56f486977">
       <div></div>
       <input type="hidden" name="in_reply_to" value="Message_2687">
       <input type="hidden" name="subject" value="fix formatting of 
positional-only parameters in inspect.Signature">
       <input type="hidden" name="message_only" value="1">
       <input type="submit" value="Send Message">
       <input type="button" value="Discard" name="discard">
       <input type="checkbox" name="send_mail" value="1" 
id="message-reply-send-mail-0" checked="checked">
       <label for="message-reply-send-mail-0">Send mail to reviewers</label>
     </form>

And here is what browser (Chrome, again) actually had sent to the server:

 1.
    message:
    Test
 2.
    xsrf_token:
    769e2ad622940b39cde28df56f486977
 3.
    in_reply_to:
    Message_2687
 4.
    subject:
    fix formatting of positional-only parameters in inspect.Signature
 5.
    message_only:
    1
 6.
    send_mail:
    1


So, the JS code is clearly working.

Yury