Last Comment Bug 773741 - Cannot resize elements in Fennec that have the CSS resize property
: Cannot resize elements in Fennec that have the CSS resize property
: testcase
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: ARM Android
-- normal (vote)
: mozilla17
Assigned To: Wesley Johnston (:wesj)
: Jet Villegas (:jet)
: 775151 (view as bug list)
Depends on:
Blocks: 553576
  Show dependency treegraph
Reported: 2012-07-13 11:29 PDT by Mardeg
Modified: 2014-06-16 12:51 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Testcase (494 bytes, text/html)
2012-07-13 11:29 PDT, Mardeg
no flags Details
Patch v1 (11.21 KB, patch)
2012-07-18 15:08 PDT, Wesley Johnston (:wesj)
enndeakin: review+
Details | Diff | Splinter Review
Patch v2 (11.61 KB, patch)
2012-07-19 17:12 PDT, Wesley Johnston (:wesj)
enndeakin: review+
Details | Diff | Splinter Review
tests (8.68 KB, patch)
2012-07-26 14:34 PDT, Wesley Johnston (:wesj)
enndeakin: review+
Details | Diff | Splinter Review

Description User image Mardeg 2012-07-13 11:29:35 PDT
Created attachment 641974 [details]

Tested on today's Firefox mobile nightly trunk on Galaxy Nexus, Android 4.1.1 Jellybean.
The grippy appears but touching/dragging it does nothing in either the attached testcase or the linked example.
Comment 1 User image Mardeg 2012-07-13 11:55:11 PDT
Confirmed in IRC on 2.3.7 Gingerbread and 4.0.4 Ice-Cream Sandwich.
Comment 2 User image microrffr 2012-07-18 11:04:19 PDT
good news.
Comment 3 User image Mardeg 2012-07-18 11:13:54 PDT
*** Bug 775151 has been marked as a duplicate of this bug. ***
Comment 4 User image Wesley Johnston (:wesj) 2012-07-18 15:08:40 PDT
Created attachment 643612 [details] [diff] [review]
Patch v1

This works well, but its tough to tap on our current resizers. Maybe we need to increase their tappable size, or talk to ux about redesigning them.

Also, I'm betting you ask for tests.....
Comment 5 User image Neil Deakin 2012-07-19 10:48:39 PDT
Comment on attachment 643612 [details] [diff] [review]
Patch v1

>+nsBoxFrame::GetEventPoint(nsGUIEvent* aEvent, nsPoint &aPoint) {
>+  nsIntPoint refPoint;
>+  bool res = GetEventPoint(aEvent, refPoint);
>+  aPoint = nsLayoutUtils::GetEventCoordinatesRelativeTo(aEvent, refPoint, this);
>+  return res;
>+nsBoxFrame::GetEventPoint(nsGUIEvent* aEvent, nsIntPoint &aPoint) {
>+  nsresult rv;
>+  if (aEvent->eventStructType == NS_TOUCH_EVENT) {
>+    rv = GetTouchPoint(static_cast<nsTouchEvent*>(aEvent), aPoint);
>+    if (NS_FAILED(rv))
>+       return false;

GetTouchPoint returns bool not nsresult.

You could also probably combine GetEventPoint and GetTouchPoint into one method instead. They could also be protected methods or even static.
Comment 6 User image Wesley Johnston (:wesj) 2012-07-19 17:12:06 PDT
Created attachment 644074 [details] [diff] [review]
Patch v2

Update nsBoxFrame methods.
Comment 7 User image Neil Deakin 2012-07-20 05:33:25 PDT
The new test file is missing from the patch.
Comment 8 User image Wesley Johnston (:wesj) 2012-07-26 14:34:22 PDT
Created attachment 646347 [details] [diff] [review]

This updates the already existing tests to use touch events as well. It also adds some helpers to EventUtils for (single) touch events. Pushed to try:
Comment 9 User image Wesley Johnston (:wesj) 2012-07-26 15:46:25 PDT
Whoops. I didn't realize the test_resizer_touchevents.xul bit was in there. Will pull it and push to try again.
Comment 10 User image Wesley Johnston (:wesj) 2012-07-27 11:47:39 PDT
Thanks neil. Second try push is still running as well, but looks ok so far:
Comment 11 User image Neil Deakin 2012-07-27 11:47:55 PDT
Comment on attachment 646347 [details] [diff] [review]

> function doTheTest() {
>+ = resizeTypes[currentResize];
>+  runTest(pointerTypes[currentPointer]);

The test used to perform an iteration using the default value for 'resize'. (The comment still describes this behaviour and is now inaccurate). It would be nice to maintain this. You could do this by not setting 'resize' for the first pass, add an extra value to resizeTypes, and skip this for touch events.

Note You need to log in before you can comment on or make changes to this bug.