Closed Bug 559440 Opened 10 years ago Closed 10 years ago

Clientwidth/-height check instead of offsetwith/-height should be used for panning

Categories

(Firefox for Android Graveyard :: Panning/Zooming, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: mfinkle)

Details

(Keywords: testcase)

Attachments

(2 files)

Attached file testcase
This is related to bug 545970.

See testcase, the content inside the black bordered box should be scrollable in itself. But this doesn't work currently.

The patch here: http://hg.mozilla.org/mobile-browser/rev/88653628be66
should probably use clientWidth/clientHeight instead of offsetWidth/offsetHeight.
Or perhaps getBoundingClientRect(), so xul documents like http://www.hevanet.com/acorbin/xul/top.xul could get panning support?
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Attached patch patchSplinter Review
Here's a patch to use clientHeight/clientWidth.  It fixes the testcase, and still works on the testcase from bug 556414.

getBoundingCLientRect does not work here; it seems to return the offsetHeight/offsetWidth rather than clientHeight/clientWidth.  The XUL example does not work because this method explicitly checks for instanceof HTMLElement - not sure why.
Attachment #439124 - Flags: review?(mark.finkle)
Oh, I see. I guess getBoundingCLientRect is more or less equivalent to offset values then.

The instanceof HTMLElement check is probably because offsetXXX properties were only working for HTML elements, not XUL elements. ClientXXX properties are also defined on XUL, though, so you probably could add that a XULElement check.
Btw, you might want to consider bug 548123. Although, it's not a big deal and Arvad isn't even sure about how much of a bug it is.
Comment on attachment 439124 [details] [diff] [review]
patch

Working on gmail, google reader and the testcase
Attachment #439124 - Flags: review?(mark.finkle) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/3f254ac9bb36
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
verified FIXED On builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.5pre) Gecko/20100420 Namoroka/3.6.5pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:1.9.3a5pre) Gecko/20100420 Namoroka/3.7a5pre Fennec/1.1a2pre
Status: RESOLVED → VERIFIED
Flags: in-litmus?
what testcase verifies this fix?   does https://litmus.mozilla.org/show_test.cgi?id=11559 suffice?
You can use the testcase in this bug to verify the bug. I've done that already with a windows 1.9.2 build of 2010-05-12 (that should suffice actually, but I know that the procedure is to verify it on the n900 device itself).
I think that is also how Aakashd verified the bug.

I don't think https://litmus.mozilla.org/show_test.cgi?id=11559 covers this particular bug. An automated test could probably be made out of this bug's testcase.
Flags: in-testsuite?
I'm reassigning this to myself for adding litmus testcases for this (aakashd discussed this on irc with blassey and ted on #mobile).
Assignee: mbrubeck → martijn.martijn
Assignee: martijn.martijn → mark.finkle
Flags: in-litmus? → in-litmus?(martijn.martijn)
Flags: in-litmus?(martijn.martijn)
You need to log in before you can comment on or make changes to this bug.