523373 - [Regression] _criticalRect is empty and throws in Rect.center()

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

* Go to the Add-on Manager
* Search for an add-on
* Select a returned add-on
* Press "Go to Page" button

"Empty rectangles do not have centers" starts getting thrown from Rect.center()

Comment 1

[Benjamin Stover (:stechz)]

Attachment: Fix

Since empty rectangles are possible, criticalRect is now never null.

Comment 2

[Benjamin Stover (:stechz)]

Attachment: Fix missed rectangle checks

Comment 3

[Roy Frostig :froystig]

Comment on attachment 407384 [details] [diff] [review]
Fix missed rectangle checks

This seems to correct all the regressions caused by the issue best described as: "Rect operations no longer give null as a sentinel value meaning 'empty', but instead make a Rect whose isEmpty() method returns true", which is the general statement of the bug filed here.

It's very difficult to be entirely sure that the patch corrects *all* cases where we used to rely on a null value to mean "empty" but no longer can (some cases might be obscure and hidden), but these are at least big common ones, if not all of them.  In particular, it does appear to fix all those associated with the critical rect.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
https://hg.mozilla.org/mobile-browser/rev/550540aec3bd

Comment 8

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2b1pre) Gecko/20091021 Fennec/1.0a4pre

and

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091021
Fennec/1.0b5pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091021
Fennec/1.0b5pre

Comment 9

[Aakash Desai [:aakashd]]

litmus testcase https://litmus.mozilla.org/show_test.cgi?id=11573 created to regression test this bug.