791647 - Various metrofx front-end fixes

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[Jim Mathies [:jimm]]

Single bug for simplicity sake.

Comment 1

[Jim Mathies [:jimm]]

Attachment: hiding tray bug

STR:

1) launch browser
2) quickly swipe the address bar down
3) wait a second, the tray will hide

We have a delayed auto-hide that kicks in after a naviation. It currently isn't reset on swipe.

Comment 2

[Jim Mathies [:jimm]]

Attachment: context menu twilight zone

STR:

1) press-hold an image or link in content
2) when the context menu comes up, tap off the menu in content to hide the context menu without selecting an option
3) press-hold an image or link in content

result: the context menu doesn't display. This is because we check popupState before the display, which doesn't get reset if you don't choose an option.

Comment 3

[Jim Mathies [:jimm]]

Attachment: css changes

Some background: when text selection in content is invoked through a tap-hold on text, a transparent overlay is displayed which holds the selection monocles and can also hold some debug related divs when selection debugging is enabled. This overlay had an issue in that it trapped input events. At the time I landed selection I had to add various work arounds for this. This weekend I was reading about css pointer-events - 

https://developer.mozilla.org/en-US/docs/CSS/pointer-events

and realized it could totally fix this. So I've added these css rules to the overlay.

In this same patch I've removed the old fennec "display a block around things that are clicked" which IMHO isn't needed on tablets and tends to junk up content when interacting with it.

Comment 4

[Jim Mathies [:jimm]]

Attachment: selection / context menu fixes

This is a set of fixes that isn't easy to separate out. 

1) with the css changes above, this removes various work arounds in SelectionHelperUI.js for the "overlay capturing events" problem, which are no longer needed.
2) Smarter interaction when tap-holding on text in form inputs related to invoking selection vs. bringing up the context menu.
3) Fixes a bug where the rect being passed via _cache data from SelectionHandler to SelectionHelperUI wasn't getting serialized properly. (It was an nsIDOMRect, which I guess message manager can't deal with.)

Comment 5

[Jim Mathies [:jimm]]

Attachment: content events conflicts

In input.js, we capture mousemove, mousedown, and mouseup events. In the treatmouseastouch environment, we use these to simulate tap events. However in the real tablet environment we just capture and stop these events since the tablet relies on gesture input generated by the backend for tap events.

In fennec we can fire mousemove, mousedown, and mouseup at content without worrying about capture listeners in chrome interfering since events weren't forwarded automatically to capture listeners in input.js. But in metro these listeners do get called, and as such input.js was interfering with simulated mouse clicks generated in Content.js. 

This fix addresses this by better filtering events received by capture listeners in input.js. In cases where the event targets the event overlay, we process the events. If however the events are targeted directly at content elements, we ignore them.

This greatly improves our interaction with content when using touch input on the tablet. There were a lot of cases where the proper mouse event sequences weren't getting delivered which caused content to react strangely.

Comment 6

[Jim Mathies [:jimm]]

Attachment: debug paint times code

Simple addition for timing paints when scrolling. I've been looking at jank in bug 789746 a little bit. This is useful in testing.

Comment 7

[Jim Mathies [:jimm]]

Attachment: rollup

rollup, including the patch in bug 791041 which all of these were built on top of.

Comment 8

[Jim Mathies [:jimm]]

Attachment: SizeChange handling for selection

STR:

1) enable auto-rotation on your tablet
2) select some text in the page
3) rotate

This patch fixes the misplaced monocle problem.

Comment 9

[Jim Mathies [:jimm]]

Comment on attachment 661773 [details] [diff] [review]
SizeChange handling for selection

Review of attachment 661773 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/contenthandlers/SelectionHandler.js
@@ +242,5 @@
> +   * recalculate the selection bounds.
> +   */
> +  _onSelectionUpdate: function _onSelectionUpdate() {
> +    if (!this._contentWindow) {
> +      this._onFail("_onSelectionMoveStart was called without proper view set up");

oops, will update this copy paste error locally.

Comment 10

[Jim Mathies [:jimm]]

Attachment: content events conflicts

Minor update to this patch. In the InputSourceHelper we can't ignore every event that isn't targeted at the overlays. We only want to do this when we are in imprecise mode. Touch mouse events targeted at content when we are in precise mode should flip to imprecise mode.

Comment 11

[Jim Mathies [:jimm]]

Attachment: rollup

Comment 12

[Frank Yan (:fryn)]

Comment on attachment 661742 [details] [diff] [review]
context menu twilight zone

Review of attachment 661742 [details] [diff] [review]:
-----------------------------------------------------------------

r+, since the patch won't hurt regardless of the answer to my question.

::: browser/metro/base/content/BrowserTouchHandler.js
@@ +256,5 @@
>        case "PopupChanged":
>        case "CancelTouchSequence":
>          this._clearPendingMessages();
> +        if (!aEvent.detail)
> +          ContextMenuUI.reset();

What does the value of aEvent.detail mean here?

Comment 13

[Frank Yan (:fryn)]

Comment on attachment 661813 [details] [diff] [review]
content events conflicts

Review of attachment 661813 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/input.js
@@ +180,5 @@
> +              Util.dumpLn("MouseModule, ignoring:", aEvent.type, aEvent.target);
> +          }
> +          break;
> +        }
> +        

Nit: whitespace

@@ +1457,5 @@
> +    return target && ("classList" in target &&
> +                      (target.classList.contains("inputHandler") ||
> +                       target.classList.contains("selection-overlay")));
> +  },
> +

Events are so messy that we have to define this twice. <sigh/>

Comment 14

[Jim Mathies [:jimm]]

(In reply to Frank Yan (:fryn) from comment #12)
> 
> What does the value of aEvent.detail mean here?

If it's null the popup is being hidden.

Comment 15

[Jim Mathies [:jimm]]

(In reply to Frank Yan (:fryn) from comment #13)
> 
> Events are so messy that we have to define this twice. <sigh/>

I can move this into Utils.

Comment 16

[Frank Yan (:fryn)]

(In reply to Jim Mathies [:jimm] from comment #15)
> (In reply to Frank Yan (:fryn) from comment #13)
> > 
> > Events are so messy that we have to define this twice. <sigh/>
> 
> I can move this into Utils.

For this particular instance of duplication, it's not a big deal either way. :)

Comment 17

[Jim Mathies [:jimm]]

(In reply to Frank Yan (:fryn) from comment #16)
> (In reply to Jim Mathies [:jimm] from comment #15)
> > (In reply to Frank Yan (:fryn) from comment #13)
> > > 
> > > Events are so messy that we have to define this twice. <sigh/>
> > 
> > I can move this into Utils.
> 
> For this particular instance of duplication, it's not a big deal either way.
> :)

yeah no big deal. I went ahead and consolidated it in Util.

Comment 18

[Matt Brubeck (:mbrubeck)]

Resolving bugs in the Firefox for Metro product that are fixed on the elm branch.  Sorry for the bugspam.  Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.