Last Comment Bug 380094 - make mousethrough more generally useful
: make mousethrough more generally useful
Status: RESOLVED WONTFIX
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla1.9alpha5
Assigned To: Chris Toshok
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-08 14:17 PDT by Chris Toshok
Modified: 2007-06-18 17:15 PDT (History)
6 users (show)
asqueella: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
first pass (8.24 KB, patch)
2007-05-08 14:22 PDT, Chris Toshok
roc: review+
roc: superreview+
Details | Diff | Splinter Review
round two (8.23 KB, patch)
2007-05-08 19:39 PDT, Chris Toshok
no flags Details | Diff | Splinter Review

Description Chris Toshok 2007-05-08 14:17:05 PDT
mousethrough would be extremely useful in more cases than it's used now.  Imagine translucent divs which overlay the actual user interface without keeping the user from interacting with that interface.
Comment 1 Chris Toshok 2007-05-08 14:22:15 PDT
Created attachment 264171 [details] [diff] [review]
first pass

Move the mousethrough handling up to nsFrame from ns{Leaf}BoxFrame, and compute it when we need it instead of updating it only when the attribute is changed (and stuffing the result in an instance variable).
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-08 15:54:10 PDT
Comment on attachment 264171 [details] [diff] [review]
first pass

+  virtual PRBool GetMouseThrough() const;

This does not need to be virtual.

Post a new version of the patch with that change, then mark this [checkin needed] and get someone to land it.
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-05-08 15:58:40 PDT
Actually, this is slightly annoying because it means if you set mousethrough on the root element then event targeting becomes O(N^2) in the depth of the frame tree. But I don't think that's worth worrying about.
Comment 4 Chris Toshok 2007-05-08 19:39:17 PDT
Created attachment 264208 [details] [diff] [review]
round two

made nsFrame::GetMouseThrough non-virtual.
Comment 5 Nickolay_Ponomarev 2007-05-11 04:47:06 PDT
Out of interest, can you describe in plain words what this patch changes for the developer? Is this something that needs changes in the documentation? Can this be tested automatically via mochitest?
Comment 6 Nickolay_Ponomarev 2007-05-13 09:01:09 PDT
Checked in, but I'm still interested in the answers to the above questions.

mozilla/layout/generic/nsFrame.cpp              3.730
mozilla/layout/generic/nsFrame.h                3.266
mozilla/layout/xul/base/src/nsBoxFrame.cpp      1.328
mozilla/layout/xul/base/src/nsBoxFrame.h        1.115
mozilla/layout/xul/base/src/nsLeafBoxFrame.cpp  1.61
mozilla/layout/xul/base/src/nsLeafBoxFrame.h    1.29
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-13 10:04:20 PDT
Why is this an attribute rather than a CSS property?  I really don't like attribute handling in frames, and adding a proprietary attribute to all languages without any prefixing seems pretty extreme.
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-13 10:17:02 PDT
That said, SVG already defines a CSS property for this:
http://www.w3.org/TR/SVG11/interact.html#PointerEventsProperty
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-13 10:19:11 PDT
(Although the default value for pointer-events doesn't really match the default value we'd want for non-SVG content for compatibility, which is 'visible' rather than 'visiblePainted'.  But we could work around that with a rule in svg.css, I think.)
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-13 10:20:10 PDT
(Actually, we'd have to work around it with a new value (-moz-auto?) since it's inherited.)
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-13 10:28:52 PDT
I filed bug 380573 on that.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-31 23:54:49 PDT
I think we should back this patch out, since I think adding a new attribute to every element in every namespace is something we should be very hesitant to do.  (As far as I know, this is the first time we've done it, although we discussed doing it for 'id'.)
Comment 13 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-05-31 23:56:00 PDT
(Well, the first time we've done it for things other than elements with XUL display types.)
Comment 14 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2007-06-01 02:03:07 PDT
OK sure. I'll back it out when the tree reopens.
Comment 15 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-06-18 17:13:29 PDT
Backed out.
Comment 16 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-06-18 17:15:11 PDT
So, as I said above (and I think in email as well), I don't think we want to do this -- I'd vastly prefer the approach in bug 380573.  Sorry for all the confusion here.

Note You need to log in before you can comment on or make changes to this bug.