1683030 - Audit ContentEventHandler::ConvertToRootRelativeOffset for fission world and probably rename it properly

Status: NEW

(Core :: DOM: UI Events & Focus Handling, task, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

So looks like what ConvertToRootRelativeOffset is supposed to be is converting app units in different pres shells, but such kind of situation should only happens with E10S as far as I can tell (or maybe happens on about: pages as well?). In the fission era, the function name and what the function does are quite confusing.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)

So looks like what ConvertToRootRelativeOffset is supposed to be is converting app units in different pres shells, but such kind of situation should only happens with E10S as far as I can tell (or maybe happens on about: pages as well?).

No, this is no longer true since bug 1261671. The function factors cumulative transform values up to the in-process root pres context. So we should revise the function to make it work inside OOP iframes.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Oops sorry for the confusion. the function was not only for E10S in the first place. :)

Comment 3

[Hiroyuki Ikezoe (:hiro)]

As I commented in bug 1649447 comment 12, ConvertToRootRelativeOffset should work as it is. But it should be renamed.

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

As I commented in bug 1649447 comment 12, ConvertToRootRelativeOffset should work as it is. But it should be renamed.

This bug is nice to have, but doesn't need to block Fission MVP if it is only about renaming the function.