Closed Bug 1089090 Opened 10 years ago Closed 9 years ago

[e10s] Custom CSS cursors don't work in remote pages

Categories

(Core :: DOM: Content Processes, defect)

Product:
Core
Component:
DOM: Content Processes
Type:
defect
Priority:
Not set
Severity:
normal
Points:
3

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla41
Iteration:
41.1 - May 25
Tracking Flags:
Tracking Status
e10s m8+ ---
firefox41 --- verified

People

(Reporter: dw-dev, Assigned: enndeakin)

References

(
URL
)

Details

Attachments

(2 files, 1 obsolete file)

zoompage-cursor-expand.png
227 bytes, image/png
Details
Implement custom cursors, v2, fix some build issues
11.62 KB, patch
jimm
: review+
Details | Diff | Splinter Review 
I have just completed migrating one of my add-ons (Zoom Page) to e10s.  Everything works fine, EXCEPT for CSS styling of the cursor in e10s windows.

Zoom Page (when viewing a standalone image) has always applied CSS styling to the <img> element to set the width/height, margin, padding, background color (for transparent images) and the cursor style (various magnifying glass icons).

All of this CSS styling still works in e10s windows, EXCEPT for the cursor styling.  The results are the same whether the styling is applied from the chrome process (using CPOW's) or from the content process.
Just to clarify, the cursor always remains styled as the default cursor.
tracking-e10s: --- → ?
Summary: [e10s] CSS styling of cursor does not work → [e10s] "Zoom Pag addon" does not work
Although this bug causes one feature (magnifying glass cursor icons) in Zoom Page not to work, I am pretty certain that this is a bug in Firefox, not in Zoom Page.
Summary: [e10s] "Zoom Pag addon" does not work → [e10s] "Zoom Page addon" does not work (cursor styling)
Blocks: e10s-addons
Component: General → Extension Compatibility
Keywords: addon-compat
https://addons.mozilla.org/en-US/firefox/addon/zoom-page/
URL: https://addons.mozilla.org/en-US/fire...
Summary: [e10s] "Zoom Page addon" does not work (cursor styling) → [e10s] "Zoom Page" addon's CSS styling works correctly, but cursor styling does not work in chrome or content process
mossop is going to have a look at this before our next triage
Flags: needinfo?(dtownsend+bugmail)
dw-dev: are you talking about customized cursors that uses one of the standard values, or are these instances of custom cursors set with a URL?

On this test page, using e10s, I see that the standard cursors work: https://developer.mozilla.org/en-US/docs/Web/CSS/cursor

So I'm guessing it's the customized URL property that doesn't work properly
Flags: needinfo?(dw-dev)
Yes, the problem occurs when the cursor is set using a data encoded URL.

For example, if applied from the chrome process, the code is:

var cursoricon = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAABGdBTUEAALGPC/xhBQAAAAlwSFlzAAAOwAAADsABataJCQAAABp0RVh0U29mdHdhcmUAUGFpbnQuTkVUIHYzLjUuMTFH80I3AAAAX0lEQVQ4T93MUQrAMAgDUO9/6W7BiLiArmVfe+BHE63d1uaI9RZ2/aRiPcOun1SsE7ImF6wTsiYXrPPwOYFvwfqDDwKyJhesE7ImF6xn2PWTivUMu35SRbEzx/73idkF9luJhTR/hQAAAAAASUVORK5CYII=";

var style = browser.contentDocument.createElement("style");
style.id = "zoompage-style-cursor";
style.type = "text/css";
style.textContent = "body > img { cursor: url(" + cursoricon + ") 6 6,default !important; }"; browser.contentDocument.childNodes[0].childNodes[0].appendChild(style);  /* append to <head> */

In this case the 'cursoricon' is a square magnifying glass containing a plus sign.  See the attached icon.png original image from which the data encoded version is derived.
Flags: needinfo?(dw-dev)
magnifying glass cursor icon
Doesn't appear to be add-on specific, custom CSS cursors just don't work in remote pages.
Component: Extension Compatibility → CSS Parsing and Computation
Flags: needinfo?(dtownsend+bugmail)
Keywords: addon-compat
OS: Windows 8 → All
Product: Firefox → Core
Hardware: x86_64 → All
Summary: [e10s] "Zoom Page" addon's CSS styling works correctly, but cursor styling does not work in chrome or content process → [e10s] Custom CSS cursors don't work in remote pages
Version: 35 Branch → Trunk
Blocks: e10s
No longer blocks: e10s-addons
Component: CSS Parsing and Computation → Widget
The code that handles this is that EventStateManager::UpdateCursor gets the cursor from layout, and then calls EventStateManager::SetCursor, which in turn calls nsIWidget::SetCursor.

Not sure if the remoting (presuming it's necessary) should live in the Widget code or in EventStateManager, but putting bug in Widget as an initial guess.
This should already work but I guess there are bugs, we forward SetCursor calls from content's PuppetWidget over PBrowser.

http://mxr.mozilla.org/mozilla-central/source/dom/ipc/PBrowser.ipdl#248
http://mxr.mozilla.org/mozilla-central/source/widget/PuppetWidget.cpp#660
(In reply to Jim Mathies [:jimm] from comment #10)
> This should already work but I guess there are bugs, we forward SetCursor
> calls from content's PuppetWidget over PBrowser.
> 
> http://mxr.mozilla.org/mozilla-central/source/dom/ipc/PBrowser.ipdl#248
> http://mxr.mozilla.org/mozilla-central/source/widget/PuppetWidget.cpp#660

Hmm, the value there is a uint32_t, so maybe we haven't implemented custom cursors yet.
Component: Widget → DOM: Content Processes
Assignee: nobody → ally
See Also: → 1103266
I'm not entirely sure bug 1103937 is a duplicate of this, though they are both about cursors. The behavior in that bug isn't related to custom cursors (but rather the built in ones), and is only triggered when the element that causes the cursor change is touching the edge of the window, and the mouse enters from outside the window. Here is a video of the bug in action https://vid.me/C6Td
http://jsfiddle.net/wNKcU/5/

With 2014-12-01 Nightly on OS X Yosemite

-Works in FF with e10s turned off

-Doesn't work with e10s turned on
Assignee: ally → nobody
Confirming JK's observations from Comment 15. Still true in Nightly.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch Implement custom cursors (obsolete) — — Splinter Review 
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED

    
  
Iteration: --- → 41.1 - May 25
Points: --- → 3
Flags: qe-verify+

    
    

    
  


  

        Attachment #8605386 -
        Attachment is obsolete: true

        Attachment #8606350 -
        Flags: review?(jmathies)

    
    

    
  
Comment on attachment 8606350 [details] [diff] [review]
Implement custom cursors, v2, fix some build issues

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

Testing on various custom cursor example sites, I didn't see any problems.

::: dom/ipc/PBrowser.ipdl
@@ +356,5 @@
>       *   update.
>       */
>      SetCursor(uint32_t value, bool force);
>  
> +    SetCustomCursor(nsCString cursorData, uint32_t width, uint32_t height,

Comment me please. I know this file and the content ipdl are awful about commenting, please help us fix it. :)

::: dom/ipc/TabParent.cpp
@@ +1227,5 @@
>        mTabSetsCursor = true;
> +      if (mCustomCursor) {
> +        widget->SetCursor(mCustomCursor, mCustomCursorHotspotX, mCustomCursorHotspotY);
> +      }
> +      else if (mCursor != nsCursor(-1)) {

nit - no wrap:

} else if () {

@@ +1750,5 @@
> +        size, aStride, static_cast<mozilla::gfx::SurfaceFormat>(aFormat), false);
> +      raw->GuaranteePersistance();
> +
> +      nsRefPtr<gfxDrawable> drawable = new gfxSurfaceDrawable(customCursor, size);
> +      nsCOMPtr<imgIContainer> cursorImage(image::ImageOps::CreateFromDrawable(drawable)); 

nit - white space

        Attachment #8606350 -
        Flags: review?(jmathies) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/6ae53b5b4785
Status: ASSIGNED → RESOLVED
Closed: 9 years ago

          status-firefox41:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41

    
    

    
  
I was able to reproduce this issue on Firefox 36.0a1 (2014-10-25).

Verified fixed on Firefox 41.0a1 (2015-06-01) and Firefox 41.0a2 (2015-08-01) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.10.4.
Status: RESOLVED → VERIFIED

          status-firefox41:
          fixed → verified

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: