Mechanize users - May 2013 - Mechanize::Form::Field#<=> is kind of broken

I''m looking into GitHub #307 reported by Jason. The gist of the problem
boils down to:

agent = Mechanize.new

agent.post some_url, [
  [''key1, ''value1''],
  [''key2'', ''value2''],
  ...
]

...sometimes results in unexpected order.

I believe the problem is that we are trying to sort the form fields
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>when
building the query string. This works just fine when the form is
parsed from the DOM, in which case we always sort them by the order they
are defined in the DOM. However when using Agent#post, there is no DOM
nodes to refer to, so we simply give up and sort the fields
randomly<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form/field.rb#L41>
(I
am pretty sure this is not a valid implementation of <=>, because when
comparing two non-DOM fields, they will simultaneously be greater than each
other).

The simple fix would be to leave out the non-DOM fields before we sort
them, and prepend or append them to the sorted (DOM-)fields array in the
order they were added. But the way <=> is implemented right now feels like
a bit of a hack so it would be nice to get that fixed as well (not too sure
how though).

Alternatively, because mechanize is primarily a "browser"
implementation
rather than a generic http library, we can just not support this and update
the docs.

Just want to get some opinions on this before I start working on it.

Godfrey
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/mechanize-users/attachments/20130505/0add32ef/attachment-0001.html>

Hey,
for a couple of years I wasn''t able to manage to unsubscribe from this
mailing list.
So hopefully you could ban me for inapropriate language.
FUCK

With my best regards
Honza


2013/5/6 Godfrey Chan <godfreykfc at gmail.com>
> I''m looking into GitHub #307 reported by Jason. The gist of the
problem
> boils down to:
>
> agent = Mechanize.new
>
> agent.post some_url, [
>   [''key1, ''value1''],
>   [''key2'', ''value2''],
>   ...
> ]
>
> ...sometimes results in unexpected order.
>
> I believe the problem is that we are trying to sort the form fields
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>when
building the query string. This works just fine when the form is
> parsed from the DOM, in which case we always sort them by the order they
> are defined in the DOM. However when using Agent#post, there is no DOM
> nodes to refer to, so we simply give up and sort the fields
randomly<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form/field.rb#L41>
(I
> am pretty sure this is not a valid implementation of <=>, because
when
> comparing two non-DOM fields, they will simultaneously be greater than each
> other).
>
> The simple fix would be to leave out the non-DOM fields before we sort
> them, and prepend or append them to the sorted (DOM-)fields array in the
> order they were added. But the way <=> is implemented right now feels
like
> a bit of a hack so it would be nice to get that fixed as well (not too sure
> how though).
>
> Alternatively, because mechanize is primarily a "browser"
implementation
> rather than a generic http library, we can just not support this and update
> the docs.
>
> Just want to get some opinions on this before I start working on it.
>
> Godfrey
>
> _______________________________________________
> Mechanize-users mailing list
> Mechanize-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mechanize-users
>


-- 
S p??n?m p?kn?ho dne
Jan Kadera
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/mechanize-users/attachments/20130506/90d8fb9c/attachment.html>

Not to argue, but isn''t the whole point of the post method to post a
form
that doesn''t exist on the page? Mind you, I JUST started using Ruby and
Mechanize, so I really have no clue what it''s intended purpose is :)

Regardless, I was able to resolve the issue temporarily by removing the
sort
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>
as
you suggested as well as the other sort
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L296>.
Thanks for your time and suggestions Godfrey.

P.S. I''m still curious what ya''ll decide to do with this


On Mon, May 6, 2013 at 1:58 AM, Godfrey Chan <godfreykfc at gmail.com>
wrote:
> I''m looking into GitHub #307 reported by Jason. The gist of the
problem
> boils down to:
>
> agent = Mechanize.new
>
> agent.post some_url, [
>   [''key1, ''value1''],
>   [''key2'', ''value2''],
>   ...
> ]
>
> ...sometimes results in unexpected order.
>
> I believe the problem is that we are trying to sort the form fields
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>when
building the query string. This works just fine when the form is
> parsed from the DOM, in which case we always sort them by the order they
> are defined in the DOM. However when using Agent#post, there is no DOM
> nodes to refer to, so we simply give up and sort the fields
randomly<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form/field.rb#L41>
(I
> am pretty sure this is not a valid implementation of <=>, because
when
> comparing two non-DOM fields, they will simultaneously be greater than each
> other).
>
> The simple fix would be to leave out the non-DOM fields before we sort
> them, and prepend or append them to the sorted (DOM-)fields array in the
> order they were added. But the way <=> is implemented right now feels
like
> a bit of a hack so it would be nice to get that fixed as well (not too sure
> how though).
>
> Alternatively, because mechanize is primarily a "browser"
implementation
> rather than a generic http library, we can just not support this and update
> the docs.
>
> Just want to get some opinions on this before I start working on it.
>
> Godfrey
>
> _______________________________________________
> Mechanize-users mailing list
> Mechanize-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mechanize-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/mechanize-users/attachments/20130506/ee18dc68/attachment.html>

Yes, you are using the method correctly as intended. And I tend to agree that
because its part of the public API this needs to be fixed. Q
?
Sent from Mailbox for iPhone

On Mon, May 6, 2013 at 4:45 AM, Jason Barnett <jason.w.barnett at
gmail.com>
wrote:
> Not to argue, but isn''t the whole point of the post method to post
a form
> that doesn''t exist on the page? Mind you, I JUST started using
Ruby and
> Mechanize, so I really have no clue what it''s intended purpose is
:)
> Regardless, I was able to resolve the issue temporarily by removing the
> sort
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>
> as
> you suggested as well as the other sort
>
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L296>.
> Thanks for your time and suggestions Godfrey.
> P.S. I''m still curious what ya''ll decide to do with this
> On Mon, May 6, 2013 at 1:58 AM, Godfrey Chan <godfreykfc at
gmail.com> wrote:
>> I''m looking into GitHub #307 reported by Jason. The gist of
the problem
>> boils down to:
>>
>> agent = Mechanize.new
>>
>> agent.post some_url, [
>>   [''key1, ''value1''],
>>   [''key2'', ''value2''],
>>   ...
>> ]
>>
>> ...sometimes results in unexpected order.
>>
>> I believe the problem is that we are trying to sort the form fields
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>when
building the query string. This works just fine when the form is
>> parsed from the DOM, in which case we always sort them by the order
they
>> are defined in the DOM. However when using Agent#post, there is no DOM
>> nodes to refer to, so we simply give up and sort the fields
randomly<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form/field.rb#L41>
(I
>> am pretty sure this is not a valid implementation of <=>, because
when
>> comparing two non-DOM fields, they will simultaneously be greater than
each
>> other).
>>
>> The simple fix would be to leave out the non-DOM fields before we sort
>> them, and prepend or append them to the sorted (DOM-)fields array in
the
>> order they were added. But the way <=> is implemented right now
feels like
>> a bit of a hack so it would be nice to get that fixed as well (not too
sure
>> how though).
>>
>> Alternatively, because mechanize is primarily a "browser"
implementation
>> rather than a generic http library, we can just not support this and
update
>> the docs.
>>
>> Just want to get some opinions on this before I start working on it.
>>
>> Godfrey
>>
>> _______________________________________________
>> Mechanize-users mailing list
>> Mechanize-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/mechanize-users
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/mechanize-users/attachments/20130506/306af625/attachment-0001.html>

Glad this was fixed!
https://github.com/sparklemotion/mechanize/commit/ef66a02bf988b1cf647121f436a026223ef5cf5d


On Mon, May 6, 2013 at 8:15 AM, Godfrey Chan <godfreykfc at gmail.com>
wrote:
> Yes, you are using the method correctly as intended. And I tend to agree
> that because its part of the public API this needs to be fixed. Q
> ?
> Sent from Mailbox <https://bit.ly/SZvoJe> for iPhone
>
>
> On Mon, May 6, 2013 at 4:45 AM, Jason Barnett <jason.w.barnett at
gmail.com>wrote:
>
>> Not to argue, but isn''t the whole point of the post method to
post a form
>> that doesn''t exist on the page? Mind you, I JUST started using
Ruby and
>> Mechanize, so I really have no clue what it''s intended purpose
is :)
>>
>> Regardless, I was able to resolve the issue temporarily by removing the
>> sort
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>
as
>> you suggested as well as the other sort
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L296>.
>> Thanks for your time and suggestions Godfrey.
>>
>> P.S. I''m still curious what ya''ll decide to do with
this
>>
>>
>> On Mon, May 6, 2013 at 1:58 AM, Godfrey Chan <godfreykfc at
gmail.com>wrote:
>>
>>> I''m looking into GitHub #307 reported by Jason. The gist
of the problem
>>> boils down to:
>>>
>>> agent = Mechanize.new
>>>
>>> agent.post some_url, [
>>>   [''key1, ''value1''],
>>>   [''key2'', ''value2''],
>>>   ...
>>> ]
>>>
>>> ...sometimes results in unexpected order.
>>>
>>> I believe the problem is that we are trying to sort the form fields
here<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form.rb#L259>when
building the query string. This works just fine when the form is
>>> parsed from the DOM, in which case we always sort them by the order
they
>>> are defined in the DOM. However when using Agent#post, there is no
DOM
>>> nodes to refer to, so we simply give up and sort the fields
randomly<https://github.com/sparklemotion/mechanize/blob/master/lib/mechanize/form/field.rb#L41>
(I
>>> am pretty sure this is not a valid implementation of <=>,
because when
>>> comparing two non-DOM fields, they will simultaneously be greater
than each
>>> other).
>>>
>>> The simple fix would be to leave out the non-DOM fields before we
sort
>>> them, and prepend or append them to the sorted (DOM-)fields array
in the
>>> order they were added. But the way <=> is implemented right
now feels like
>>> a bit of a hack so it would be nice to get that fixed as well (not
too sure
>>> how though).
>>>
>>> Alternatively, because mechanize is primarily a "browser"
implementation
>>> rather than a generic http library, we can just not support this
and update
>>> the docs.
>>>
>>> Just want to get some opinions on this before I start working on
it.
>>>
>>> Godfrey
>>>
>>> _______________________________________________
>>> Mechanize-users mailing list
>>> Mechanize-users at rubyforge.org
>>> http://rubyforge.org/mailman/listinfo/mechanize-users
>>>
>>
>>
>
> _______________________________________________
> Mechanize-users mailing list
> Mechanize-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mechanize-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/mechanize-users/attachments/20130515/f96e18b6/attachment.html>