[rfc][icedtea-web] rewert of changset 757 "Simplify IcedTeaScriptableJavaObject"
aazores at redhat.com
Thu Jan 16 08:40:12 PST 2014
On 01/14/2014 10:48 AM, Jiri Vanek wrote:
> Unluckily the regressions caused by (otherwise good)
> +2013-06-21 Adam Domurad <adomurad at redhat.com>
> + * plugin/icedteanp/IcedTeaScriptablePluginObject.cc: Simplify
> + IcedTeaScriptableJavaObject
> + * plugin/icedteanp/IcedTeaScriptablePluginObject.h: Same
> were never successfully fixed, and I gave up on it :(
> This is revert of patch in tip. Please note, that this is *manual*
> revert of patch.
> When I did it , i was just 50% sure what I done. But the result of
> selected set of reproducers have persuaded me that I did it correctly.
> (three "r" attachments)
> So (any) reviwer, please (and sorry!), be double careful!-)
Well, the patch generally looks okay, and your results are convincing.
There seems to be no affect for me running the tests in Firefox, but at
least reverting the changes hasn't made anything worse as far as I can
tell. There is this that I pointed out over IRC and will repeat here though:
// If object is an array and requested "method" may be a number,
check for it first
- if ( scriptable_object->is_object_array ||
+ if ( !((IcedTeaScriptableJavaObject*) npobj)->isArray() ||
(browser_functions.intfromidentifier(name_id) < 0))
it seems to me like the value of this condition has changed. But, you
are restoring it back to how it used to be, so perhaps this is actually
what the regression was in the first place. Like I said, it seems to
make no difference in my testing, but perhaps it would be worth it if
you checked the results of removing the negation in the condition as
well. Most of the rest of the patch seems to be simple refactoring but
this appears to actually change some behaviour.
More information about the distro-pkg-dev