Return value of HandleCrossProcessEvent is ignored

RESOLVED WONTFIX

Status

()

Core
DOM: Events
RESOLVED WONTFIX
5 years ago
3 years ago

People

(Reporter: evilpie, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10slater)

Details

(Whiteboard: [e10s])

(Reporter)

Description

5 years ago
This functions seems to indicate if it did something. Maybe we can return early, otherwise we should change the return value to void. https://mxr.mozilla.org/mozilla-central/source/content/events/src/nsEventStateManager.cpp#3073
(Reporter)

Comment 1

5 years ago
This turned out to be a real issue. This was causing bug 893201.

Short IRC snippet:
16:08 <evilpie> okay seems to introduce some other problems
16:10 <evilpie> like that you can focus content with the mouse
16:13 <evilpie> felipe: seems like you added that code
16:15 <felipe> evilpie: yeah, I don't recall the details now, but if I skipped PostHandleEvent it broke various other things
16:16 <evilpie> maybe we should just skip in certain cases
16:16 <evilpie> or eg for tab, check if we send it to content and only skip it then
16:16 <evilpie> but it seem wrong to handle it twice
16:16 <felipe> maybe you can still use the return value in some useful way, but I doubt it will be just to return early there
16:16 <evilpie> I was looking at this, because tab was handled in content and chrome
16:16 <evilpie> so I could eg. special case this
16:17 <evilpie> maybe there are other cases where we want to skip
16:17 <felipe> which case did you find that gets fixed by skipping it?
16:17 <evilpie> tab focus handling
16:18 <evilpie> at the moment you can't tab from chrome to content
16:18 <felipe> i don't think it's too wrong to handle it twice.. parent and chrome have a totally separate event dispatching/bubbling
16:19 <felipe> but as you mention, some cases will need to be aware of that
16:19 <evilpie> I think this is one
16:19 <felipe> (and btw, i'm just thinking out loud, not saying I'm right about this!)
16:19 <felipe> so, with that, does it work that you can tab through all of the elements in content, and then continue to tab through chrome?
(Reporter)

Updated

5 years ago
Whiteboard: [e10s]
(Reporter)

Comment 2

5 years ago
We need to look into if there are other cases where we should do something with the return value.
Depends on: 893201
(Reporter)

Updated

5 years ago
Depends on: 904865
Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".
tracking-e10s: --- → +
tracking-e10s: + → later
Whiteboard: [e10s] → [e10s][good-first-bug][mentor=felipe]

Updated

4 years ago
Whiteboard: [e10s][good-first-bug][mentor=felipe] → [e10s][good first bug][mentor=felipe]
(Assignee)

Updated

4 years ago
Mentor: felipc@gmail.com
Whiteboard: [e10s][good first bug][mentor=felipe] → [e10s][good first bug]
hi.. I would like to work on this.. Could you please tell me how to begin ?
So, I don't think this is actually a good first bug. The scope is not well defined, and while we know the  return value of HandleCrossProcessEvent is ignored, it's a lot less clear on what to do with it. So taking on this bug is probably gonna be frustrating and lead to nowhere. I'll mark this bug as wontfix for now because there are no clear problems arising from it. If something like that shows up in the future we can reopen it or fix it in a case by case basis on a separate bug.

Kapil, your desire to help is definitely appreciated! Please keep looking for other good first bugs to find another one that interests you. If you find one that has no mentor, i'll be more than happy to help you with it!
Mentor: felipc@gmail.com
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(felipc)
Resolution: --- → WONTFIX
Whiteboard: [e10s][good first bug] → [e10s]
You need to log in before you can comment on or make changes to this bug.