Botond Ballo [:botond] Reporter
	Description • 10 years ago

The relevant code in BEP.js:

  - subscribes to the following observer events:
      - "BEC:ShownModalPrompt"
      - "Activity:Success"
      - "Activity:Error"
  - listens to "visibilitychange" events
  - when any of these events is fired, resets the hover state
    by calling SetContentState(NS_EVENT_STATE_HOVER) on a dummy
    element

Porting this to C++ will take us one step closer to removing BEP.js.

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 1 • 10 years ago

nice!

	Botond Ballo [:botond] Reporter
	Comment 2 • 9 years ago

Kats, do you know if Fennec does something similar with the hover state, i.e. is this something we want to port to C++ for fennec-apz the way we ported the double-tap handling in bug 1131359?

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 3 • 9 years ago

As far as I know (and from a quick glance at the code) Fennec doesn't have any hover handling at all. And thinking about it, to me it doesn't make sense to have hover states on touch anyway since you can't actually do a "hover". I think we should just rip out this code and throw it away; if it turns out that without it we are setting hover states somewhere and not clearing them, we should fix the appropriate code to not set the hover state in the first place. Does that sound reasonable? If it turns out the hover state is intentional/legitimate then we can resurrect this code in C++ form.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 4 • 8 years ago

Kan-Ru, is BrowserElementPanning used on any platform other than B2G? Or can we delete this code as well?

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 5 • 8 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Kan-Ru, is BrowserElementPanning used on any platform other than B2G? Or can
> we delete this code as well?

Has APZ fully replaced BrowserElementPanning? The new responsive design mode in devtools is using mozbrowser so I think it is using BrowserElementPanning on touch enabled platfrom.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 6 • 8 years ago

(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #5)
> Has APZ fully replaced BrowserElementPanning?

I believe so, yeah. The only outstanding bug is this one and it deals with touch-hover states which as I said in comment 3 shouldn't be a thing really. So as far as I'm concerned we should be ok to rip this out. :jryans, do you know if the new RDM is relying on any particular behaviour from BrowserElementPanning? If it still works with the lines at [1] commented out we should be good to go.

[1] http://searchfox.org/mozilla-central/rev/8071f94c4d4b66833ad08db39565669faad94dfd/dom/ipc/preload.js#95-104

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 7 • 8 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6)
> (In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #5)
> > Has APZ fully replaced BrowserElementPanning?
> 
> I believe so, yeah. The only outstanding bug is this one and it deals with
> touch-hover states which as I said in comment 3 shouldn't be a thing really.
> So as far as I'm concerned we should be ok to rip this out. :jryans, do you
> know if the new RDM is relying on any particular behaviour from
> BrowserElementPanning? If it still works with the lines at [1] commented out
> we should be good to go.

preload.js is only used on B2G, so there's a slightly different block[1] to load these files in the desktop mozbrowser.  In any case, on macOS BrowserElementPanning is never loaded, because dom.w3c_touch_events.enabled defaults to 0 on macOS.  The file is loaded on Windows and Linux.

So, we do load this file on some desktop platforms when using RDM.  However, AFAIK we aren't using the functionality it supports.  It seems like the main result is that it emits an observer notification "browser-zoom-to-rect", but I don't see anyone listening for this notification in m-c, so no zooming actually takes place.  It's also manipulating hover states in response to a few messages, like you stated in comment 3.

For the purposes of RDM, I don't believe any of this functionality is needed, so it seems safe to remove to me!  If we want features like double tap to zoom in RDM, we can pursue something like bug 1282089 to achieve this.

[1]: https://dxr.mozilla.org/mozilla-central/rev/ea104eeb14cc54da9a06c3766da63f73117723a0/dom/browser-element/BrowserElementChild.js#61-69

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 8 • 8 years ago

Thanks. Considering this bug is actually about the hover code in BEP.js, and bug 970125 is about getting rid of BEP.js, I'm going to close this bug as wontfix, and rip out the code in bug 970125.