[CTW] Get rid of the Windows ifdefs for PDocAccessible CaretMoveEvent and FocusEvent
Categories
(Core :: Disability Access APIs, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox117 | --- | fixed |
People
(Reporter: Jamie, Assigned: Jamie)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ctw-postship])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
After bug 1831035, PDocAccessible will be unified across platforms. However, there are still some ifdefs. Two of these are FocusEvent and CaretMoveEvent. This is because on Windows, these events need to push the caret rectangle. The caret rectangle isn't used on any other platform.
There are a few paths forward:
- Do nothing; wontfix this bug. An ifdef is a bit ugly, but probably not the end of the world.
- Make the FocusEvent ifdef more like the CaretMoveEvent ifdef; i.e. just ifdef the caret rect parameter. That way, we at least don't have two separate functions handling focus events on different platforms: PDocAccesible::FocusEvent on Windows and PDocAccessible::Event everywhere else.
- Get rid of the ifdef almost everywhere except where we would pass 0s for the caret rect on non-Windows. This avoids calculating the caret rect where it won't be used, but makes most of the code cleaner.
- Just push the caret rect on all platforms. This unifies everything across platforms, at the cost of something that won't ever get used on non-Windows.
Assignee | ||
Comment 1•1 year ago
|
||
Eitan, I think these ifdefs bother you more than they do me. Do you have any preference as to which option we go with here?
Assignee | ||
Comment 2•1 year ago
|
||
Personally, I'd probably prefer option 3 or 4. 3 avoids pointless computation, but that might be premature optimisation.
Comment 3•1 year ago
|
||
Can't we just pass an empty rect to CaretMoveEvent on non-windows instead of having a different protocol? I would like us to have the same protocol everywhere, but maybe utilize it differently on different platforms to avoid expensive things.
Honestly, I don't have strong opinions here and can go whatever way you choose since you are the great unifier. I think we both share the goal of simplifying this stuff and de-fragmenting it, so whatever path you choose is great.
We really just need to pick some balance with regard to simple code base, unified test outcomes across platforms, and not spending extra resources on things that aren't used in other platforms. A thing that comes to mind is the EVENT_SCROLLING
event that is not consumed on anything but android and is probably wasteful. The flip side is that it is really nice to have good browser tests that run in desktop platforms for that.
Assignee | ||
Comment 4•1 year ago
|
||
(In reply to Eitan Isaacson [:eeejay] from comment #3)
Can't we just pass an empty rect to CaretMoveEvent on non-windows instead of having a different protocol? I would like us to have the same protocol everywhere, but maybe utilize it differently on different platforms to avoid expensive things.
Yup, that's what I meant by option 3. Sorry if that wasn't clear. Sounds like we're both on the same page here; just wanted a gut check.
Assignee | ||
Comment 5•1 year ago
|
||
On Windows, focus and caret move events include the caret rectangle.
This isn't used on other platforms.
To simplify the cross-platform interface (including Platform.h), remove the ifdefs from there.
However, we use ifdefs to avoid calculating the rectangle on non-Windows platforms, instead just sending an empty rectangle.
Updated•1 year ago
|
Comment 7•1 year ago
|
||
bugherder |
Description
•