Asaf Romano (gone) Reporter
	Description • 17 years ago

a tree inside a panel doesn't respond to clicks (mac only?).

	Asaf Romano (gone) Reporter
	Comment 2 • 17 years ago

I see this on windows as well.

	David Baron :dbaron:
	Comment 3 • 17 years ago

roc and I aren't the right people to triage the blocking1.9? flag here.  Not sure how to fix that...

	Neil Deakin
	Comment 5 • 17 years ago

This bug occurs because clientX and clientY are 'unusual' when inside popups, and even more so when the popup contains a child with a widget, to the point where different values are returned for the same location just because a <deck> exists elsewhere inside a popup.

This patch makes it so that clientX/clientY within popups returns a value relative to the popup's topleft corner. Another possibility is to return the values relative to the document.

I don't really understand if this is the right patch, but it seems to work ok.

Thoughts?

	Eli Friedman Assignee
	Comment 6 • 17 years ago

Here's a possibility.  I'd need approval to land this for 1.9, though.

	Eli Friedman Assignee
	Comment 7 • 17 years ago

I guess I ought to say a bit more.  The original reason for this mess of client coordinates was that the popup code used clientX/Y in a lot of places, and expected it to be in this funny not-quite-client-coordinate system.  With the popup rewrite, that's no longer the case, so it's probably safe to just make it behave the way you'd expect even if there are popups.

There is some chance that content using popups expects the old behavior, though, so fixing it properly is potentially a breaking change.

	neil@parkwaycc.co.uk
	Comment 8 • 17 years ago

(In reply to comment #7)
>I guess I ought to say a bit more.  The original reason for this mess of client
>coordinates was that the popup code used clientX/Y in a lot of places, and
>expected it to be in this funny not-quite-client-coordinate system.  With the
>popup rewrite, that's no longer the case, so it's probably safe to just make it
>behave the way you'd expect even if there are popups.
Which is? I'm hoping that event.clientX == event.screenX - event.target.ownerDocument.documentElement.boxObject.screenX
as per the XXXbz comment in mailWidgets.xml :-)

	Neil Deakin
	Comment 9 • 17 years ago

(In reply to comment #6)
> Created an attachment (id=274069) [details]
> Fix clientX/Y
> 
> Here's a possibility.  I'd need approval to land this for 1.9, though.
> 

Yes, this is probably a better idea.

	Eli Friedman Assignee
	Comment 10 • 17 years ago

(In reply to comment #8)
> (In reply to comment #7)
> Which is? I'm hoping that event.clientX == event.screenX -
> event.target.ownerDocument.documentElement.boxObject.screenX
> as per the XXXbz comment in mailWidgets.xml :-)

My patch makes that true.

	Eli Friedman Assignee
	Comment 11 • 17 years ago

Correction: my patch makes that true except in situations where zoom is involved.  (See the discussion in m.d.t.layout.)

	Boris Zbarsky [:bzbarsky]
	Comment 12 • 17 years ago

I am so totally in favor of Eli's patch.  I do worry about extensions that might depend on the client* in popups like that, but since it would be broken pretty much whatever they tried to do with it (c.f. mailwidgets.xml), I think there aren't many, if any.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 13 • 17 years ago

Seconded.

	Eli Friedman Assignee
	Comment 14 • 17 years ago

Comment on attachment 274069 [details] [diff] [review]
Fix clientX/Y

If we're going to get this in, I should ask for review.

	Eli Friedman Assignee
	Comment 15 • 17 years ago

Checked in.  (Might be a little difficult to write tests with popups; I guess we can do it with mochitests.)

	neil@parkwaycc.co.uk
	Comment 16 • 17 years ago

So does this mean we can remove all bz's hacks such as this one:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/base/resources/content/mailWidgets.xml&rev=1.76&root=/cvsroot&mark=1711-1713#1705

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 17 • 17 years ago

I think so!

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 18 • 17 years ago

One thing I thought of belatedly: getBoundingClientRect for an element inside an SVG foreignobject uses the nearest foreignobject as the root of its coordinate system. Event client coordinates probably should do.

	Eli Friedman Assignee
	Comment 19 • 17 years ago

(In reply to comment #18)
> One thing I thought of belatedly: getBoundingClientRect for an element inside
> an SVG foreignobject uses the nearest foreignobject as the root of its
> coordinate system. Event client coordinates probably should do.

Maybe... what exactly was the justification for getBoundingClientRect returning coordinates relative to the nearest foreignobject?  I don't think we want to force web authors to check whether the target content has a foreignobject parent before using clientX/Y.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 20 • 17 years ago

The justification was that the obvious alternative behaviour --- taking the bounding rect of the SVG-transformed element(s) --- would confuse most uses of getBoundingClientRect.

I'm not sure what to do here.

	Bob Clary [:bc] (inactive)
	Comment 21 • 17 years ago

Please see bug 391024.

	Neil Deakin
	Comment 23 • 17 years ago

Checked in the test