Closed Bug 245529 Opened 17 years ago Closed 17 years ago

gtkmozembed is out of sync wrt how DOM event propagation is blocked

Categories

(Core Graveyard :: Embedding: GTK Widget, defect)

x86
Linux
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mozillabugs.philipl, Assigned: mozillabugs.philipl)

Details

(Keywords: embed, fixed1.7)

Attachments

(1 file)

The fix for bug 233142 removed the ability to block a DOM event by returning
something other than NS_OK from an event handler. Rather, the handler now must
call |stopPropagation| and |preventDefault| on the event to block it.

As such, all the gtk signal wrappers for the DOM events must be rewritten so
that they check the return code of the gtk callback and block the event if TRUE
is returned or do nothing otherwise; this preserves the gtk signal semantics.

This affects the 1.7 branch and trunk.

I'll work up a patch in the next few days, but I *really* hope this gets into
1.7, otherwise the DOM event signals will be very broken.
Flags: blocking1.7?
Here's my patch to fix the problem. It retains the correct gtk callback
semantics. As far as I can see, there's nothing to be gained from checking the
return values of the |stopPropagation| and |preventDefault| calls, so I don't.
Assignee: blizzard → philipl
Status: NEW → ASSIGNED
Attachment #149982 - Flags: superreview?(blizzard)
Attachment #149982 - Flags: review?(jst)
Comment on attachment 149982 [details] [diff] [review]
Patch to allow gtk callbacks to block event propagation again

r=jst
Attachment #149982 - Flags: review?(jst) → review+
Flags: blocking1.7? → blocking1.7+
Comment on attachment 149982 [details] [diff] [review]
Patch to allow gtk callbacks to block event propagation again

a=dbaron conditional on getting sr
Attachment #149982 - Flags: superreview?(bryner)
Attachment #149982 - Flags: superreview?(blizzard)
Attachment #149982 - Flags: approval1.7+
Attachment #149982 - Flags: superreview?(bryner) → superreview+
let's get this landed quickly.
The patch is fully approved but I don't have check-in rights, so if someone who
does could check it in, that would be great.

It should be checked into the 1.7 branch and trunk. The diff is against 1.7
branch but applies to trunk as is. I have confirmed that it is correct for both.
Fixed on trunk and 1.7 branch. Thanks for the fix, Philip!
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: fixed1.7
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.