UITour: Pointer-events should go through highlights

RESOLVED FIXED in Firefox 29

Status

()

defect
P3
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: MattN, Assigned: enndeakin)

Tracking

(Depends on 1 bug)

Trunk
Firefox 29
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29+ fixed)

Details

()

Attachments

(3 attachments)

This isn't working properly on OS X. I'm not sure about other platforms. This is preferred for the proposed Australis update flow as the user can start by opening the menu panel while the menu button is highlighted. Worst case, the user can click the "Show me more!" button.
Not working on Windows either.
OS: Mac OS X → All
Enn, we have a <panel>[1] that we want to pointer-events to go through (e.g. pointer-events: none). Do you know of a way to do this or how hard it would be to make it work?

[1] https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul?rev=0d1e734bbd0c#208
Flags: needinfo?(enndeakin)
The workaround we can fallback to is re-dispatching events from the panel to the panel's anchor since the panel normally doesn't overlap other elements much.
Assignee

Comment 4

6 years ago
There's support for having mouse events pass through panels already on Windows and Mac but not on Linux, although it's only currently available for drag feedback popups, (the translucent images that appear when dragging).

http://mxr.mozilla.org/mozilla-central/ident?i=mIsDragPopup lists places which would need to be changed to be extended to other types of popups.
Flags: needinfo?(enndeakin)
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Priority: P4 → P3
Neil, if you have time to take this bug, feel free to steal it.
Assignee: MattN+bmo → enndeakin
Assignee

Comment 6

6 years ago
In layout/xul, we allow <panel mousethrough="always"> to be used as a way to ignore mouse events on the window. We could extend it to support pointer-events but adding that doesn't really gain anything. nsXULPopupManager::GetVisiblePopups is changed so that this can be supported on all popups.

Thie patch includes an implementation of this using input shapes on gtk and fixes mouse events on Windows. Mac needs no extra fixes.
Attachment #8365157 - Flags: feedback?(MattN+bmo)
I did a quick test on OS X and found an issue with hover styling when the customize mode icon is highlighted (step 2/4 in the tour) . When I hover over the highlight, the button doesn't get the hover state like it does if I move the cursor to the side of the highlight.
Assignee

Comment 8

6 years ago
I don't see any issue here. Whenever I hover over the blue highlight, the Customize button highlights in dark grey.
(In reply to Neil Deakin from comment #8)
> I don't see any issue here. Whenever I hover over the blue highlight, the
> Customize button highlights in dark grey.

OK, I'm on Mavericks (10.9) when I'm seeing this. I guess we can go ahead with this patch. Do you want me to test on the other platforms or what kind of feedback do you want? @mousethrough="always" is fine instead of pointer-events: none.
Assignee

Comment 10

6 years ago
I just wanted to make sure the implementation matched what was expected.
Attachment #8365157 - Flags: feedback?(MattN+bmo) → feedback+
Assignee

Updated

6 years ago
Attachment #8365157 - Flags: review?(neil)
Assignee

Comment 11

6 years ago
Comment on attachment 8365157 [details] [diff] [review]
Implement mouse transparency on all platforms

windows/nsWindow.cpp/h changes
Attachment #8365157 - Flags: review?(jmathies)
Assignee

Comment 12

6 years ago
Comment on attachment 8365157 [details] [diff] [review]
Implement mouse transparency on all platforms

GTK changes. This works but not sure if there's a better way to accomplish this. I realize I should be passing rect to cairo_region_create_rectangle so I'll fix that in the next iteration.
Attachment #8365157 - Flags: review?(karlt)
Comment on attachment 8365157 [details] [diff] [review]
Implement mouse transparency on all platforms

>+    // A drag popup may be used for non-static translucent drag feedback
>+    bool isDragPopup = false;
>+    if (mPopupType == ePopupTypePanel &&
>+        mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::type,
>+                              nsGkAtoms::drag, eIgnoreCase)) {
>+      widgetData.mIsDragPopup = true;
Where does this get used? Also while I'm here, where do drag popups get created? (I couldn't find where the type="drag" attribute gets set.)

>+  // Don't pass events to popups that ignore mouse events.
>+  if (aBuilder->IsForEventDelivery() && mMouseTransparent) {
Won't the builder handle the mouse transparency case already?
Comment on attachment 8365157 [details] [diff] [review]
Implement mouse transparency on all platforms

Review of attachment 8365157 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsWindow.cpp
@@ +362,5 @@
>    mForeground           = ::GetSysColor(COLOR_WINDOWTEXT);
>  
>    mTaskbarPreview = nullptr;
>    mHasTaskbarIconBeenCreated = false;
> +  mMouseTransparent = false;

nit - could you move these two bools up to where we init the result of the boolean member variables.
Attachment #8365157 - Flags: review?(jmathies) → review+
Assignee

Comment 15

6 years ago
> Where does this get used? Also while I'm here, where do drag popups get
> created? (I couldn't find where the type="drag" attribute gets set.)

I don't think it ended up being used in Firefox currently, although it was going to be used for tab dragging.

> Won't the builder handle the mouse transparency case already?

Well, it won't work for drag popups. But maybe they should just explicitly set mousethrough then, so I expect I can now just remove the BuildDisplayList override there.
Can you tell me how to run the UITour, please?

Do I need to find login details for https://docs.google.com/a/mozilla.com/file/d/0B60Ayj_R5h_jb0gzSWtpSkpSeXM/edit ?
Flags: needinfo?(MattN+bmo)
Assignee

Comment 17

6 years ago
1. Set the browser.uitour.whitelist.add.testing preference to www-demo3.allizom.org
2. Load https://www-demo3.allizom.org/b/en-US/firefox/australis/update/

I needed to create a new profile to get it to work.
Assignee

Comment 18

6 years ago
The following xul testcase demonstrates how this works.
Flags: needinfo?(MattN+bmo)
(In reply to Neil Deakin from comment #15)
> > Where does this get used? Also while I'm here, where do drag popups get
> > created? (I couldn't find where the type="drag" attribute gets set.)
> 
> I don't think it ended up being used in Firefox currently, although it was
> going to be used for tab dragging.

How would tab dragging need the widget-level flag?


> > Won't the builder handle the mouse transparency case already?
> 
> Well, it won't work for drag popups. But maybe they should just explicitly
> set mousethrough then, so I expect I can now just remove the
> BuildDisplayList override there.
Accidental submit, sorry.

(In reply to Neil Deakin from comment #15)
> > Where does this get used? Also while I'm here, where do drag popups get
> > created? (I couldn't find where the type="drag" attribute gets set.)
> 
> I don't think it ended up being used in Firefox currently, although it was
> going to be used for tab dragging.

How would tab dragging need the widget-level flag, which gets thrown away almost immediately?

> > Won't the builder handle the mouse transparency case already?
> 
> Well, it won't work for drag popups. But maybe they should just explicitly
> set mousethrough then, so I expect I can now just remove the
> BuildDisplayList override there.

Yes, I would appreciate it if you did that.
Assignee

Comment 21

6 years ago
(In reply to neil@parkwaycc.co.uk from comment #20)
> Accidental submit, sorry.
> 
> (In reply to Neil Deakin from comment #15)
> > > Where does this get used? Also while I'm here, where do drag popups get
> > > created? (I couldn't find where the type="drag" attribute gets set.)
> > 
> > I don't think it ended up being used in Firefox currently, although it was
> > going to be used for tab dragging.
> 
> How would tab dragging need the widget-level flag, which gets thrown away
> almost immediately?
> 

Tab dragging mostly needed the mousethrough part. With the patch, type="drag" only serves to hint to gtk that the popup is a drag popup. See http://mxr.mozilla.org/mozilla-central/source/widget/gtk/nsWindow.cpp#3527 so it now doesn't do anything specific on other platforms.
FYI, we would really like this for the initial Aurora 29 release as the UITour will be used at that point (bug 966014) and this is a usability issue hindering that. Let me know if that isn't do-able.
Comment on attachment 8365157 [details] [diff] [review]
Implement mouse transparency on all platforms

Ah, I hadn't realised that there was still one consumer left. In that case, r=me if you add explicit mousethough to drag popups and remove the BuildDisplayList override.
Attachment #8365157 - Flags: review?(neil) → review+
Comment on attachment 8365157 [details] [diff] [review]
Implement mouse transparency on all platforms

>+  // true if the window should be transparent to mouse events
>+  bool          mMouseTransparent;

Can you document that this is only effective for eWindowType_popup, please?
Attachment #8365157 - Flags: review?(karlt) → review+
https://hg.mozilla.org/mozilla-central/rev/af70f4108299
https://hg.mozilla.org/mozilla-central/rev/ed29e233ee6b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Depends on: 969173
Assignee

Updated

5 years ago
Blocks: 712184
Depends on: 993366
You need to log in before you can comment on or make changes to this bug.