Complete support of CSS2/3 cursors on Gtk

VERIFIED FIXED in Future

Status

()

P3
normal
VERIFIED FIXED
19 years ago
16 years ago

People

(Reporter: michael.j.lowe, Assigned: pavlov)

Tracking

({css2, css3, platform-parity})

Trunk
Future
All
Linux
css2, css3, platform-parity
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(16 attachments)

6.11 KB, patch
Details | Diff | Splinter Review
3.99 KB, patch
Details | Diff | Splinter Review
6.55 KB, patch
Details | Diff | Splinter Review
1.32 KB, patch
Details | Diff | Splinter Review
12.78 KB, patch
Details | Diff | Splinter Review
1.28 KB, patch
Details | Diff | Splinter Review
12.78 KB, patch
Details | Diff | Splinter Review
10.20 KB, patch
Details | Diff | Splinter Review
14.10 KB, patch
Details | Diff | Splinter Review
9.95 KB, patch
Details | Diff | Splinter Review
9.64 KB, patch
Details | Diff | Splinter Review
9.70 KB, patch
Details | Diff | Splinter Review
3.01 KB, patch
Details | Diff | Splinter Review
13.73 KB, patch
Details | Diff | Splinter Review
13.73 KB, patch
Details | Diff | Splinter Review
557 bytes, patch
Details | Diff | Splinter Review
(Reporter)

Description

19 years ago
Implementation of CSS2 and CSS3 cursor properties needs to be completed on Gtk.
  The Windows implementation of these is mostly complete now.   

Work needs to be done in nsWidget::SetCursor(nsCursor aCursor) to support the 
following cases correctly:

      case eCursor_sizeWE:
      case eCursor_sizeNS:
      case eCursor_sizeNW:
      case eCursor_sizeSE:
      case eCursor_sizeNE:
      case eCursor_sizeSW:

      case eCursor_crosshair:
      case eCursor_move:
      case eCursor_help:

      case eCursor_copy: // CSS3
      case eCursor_alias:
      case eCursor_cell:
      case eCursor_grab:
      case eCursor_grabbing:
      case eCursor_spinning:
      case eCursor_context_menu:
      case eCursor_count_up:
      case eCursor_count_down:
      case eCursor_count_up_down:

Refer to the following URL for a test case of CSS2 cursors:

http://www.people.fas.harvard.edu/~dbaron/css/test/sec1801
(Reporter)

Updated

19 years ago
Keywords: css2, css3
(Reporter)

Updated

19 years ago
Keywords: pp
(Reporter)

Updated

19 years ago
Blocks: 1916
(Assignee)

Comment 1

19 years ago
i'm accepting patches.  this should be easy to do, but I'm not gonna have time
to look at it for a while.
Status: NEW → ASSIGNED
Target Milestone: --- → M30

Comment 2

19 years ago
Mass-moving all M20-M30 XPToolkit bugs to Future
Target Milestone: M30 → Future

Updated

19 years ago
Blocks: 40421

Comment 3

19 years ago
Available Gnome cursors are defined in:

http://lxr.mozilla.org/gnome/source/gtk+/gdk/gdkcursors.h

For example, GDK_HAND2

The Gnome cursors are created by calling XCreateFontCursor()

You can see what they all look like (with the exception of GDK_NUM_GLYPHS (154)) 
at:  (brace yourself, it's not a pretty sight)

http://www.tigr.net/afterstep/X/xlib/appendix/b/

We might be able to choose something reasonable from that list for each of our 
eCursor_xxx.  Or, maybe not.

Gnome is considering adding their own cursors, as we see in:

http://lxr.mozilla.org/gnome/source/gnome-libs/TODO.shtml#310

Because, "X's default cursors are butt ugly."

I suggest that we contact whoever wrote that at Gnome, and ask them to consider 
CSS2 / CSS3 cursors while they're adding some.

In the meantime, I suggest we pick from the available list.

Does anyone know where the standard for CSS2 / 3 cursors is?

This also needs fixing in nsWindow.cpp as described in bug 40421.

Comment 4

19 years ago
I'm working on a patch for this and bug 40421.

Comment 6

19 years ago
Created attachment 9306 [details] [diff] [review]
Patch to fix 40421 and get much closer on 38444

Comment 7

19 years ago
My patch should fix bug 40421 completely, support pre-defined CSS2 cursors, and
some of the CSS3 cursors.  I picked the closest available cursor.

For several of the CSS3 cursors, there is no appropriate cursor available from
XCreateFontCursor().

As yet unimplemented:

eCursor_copy, eCursor_alias, eCursor_count_up, eCursor_count_down,
eCursor_count_up_down.


CSS2 also supports loading custom cursors from URLs which doesn't seem to be
supported yet.

The CSS3 Cursor spec is at:

http://www.w3.org/TR/css3-userint#cursor
The CSS3 cursors are still in (early) working draft, and are subject to change. 
If they are implemented (which they really shouldn't be unless there's a real
need for them) then the status of the draft should be tracked closely.

Updated

19 years ago
Keywords: patch
(Assignee)

Comment 9

19 years ago
sorry I havn't responded earlier... like weeks ago...

this patch looks fine.  I'm nominating it for nsbeta2 so that I can check it in
for you.

PDT: this patch is fine and from an outside contributor that does not have a cvs
account.  Please let me check this in for him.
Keywords: nsbeta2

Comment 10

19 years ago
[nsbeta2-] not required for seamonkey beta2, but you can check in mozilla 
community contributions as long as brendan or waterson approve.
Whiteboard: [nsbeta2-]
(Assignee)

Comment 11

19 years ago
Created attachment 10278 [details] [diff] [review]
patch -u style
(Assignee)

Comment 12

19 years ago
ok, i've checked in the patch.  i'm going to leave this open until we get the
rest of the css3 cursors.  I think that we should probably make some xbms for
the cursors we need/don't like.
Keywords: nsbeta2, patch
Whiteboard: [nsbeta2-]

Comment 13

19 years ago
I've downloaded the daily snapshot of the source, and the patch has been applied 
to widget/src/gtk/nsWindow.cpp

However, the patch put the comment in the wrong place in the function 
nsWindow::Setcursor.  The 3 line comment should be just above the 
switch(aCursor) statement.

There were two files patched in that diff I uploaded, and the other one did not 
get the patch applied at all.  It's identical patch applied to nsWidget.cpp in 
the same directory.

If you will, please move that comment down a few lines, and patch the other 
file.

Thanks!

Comment 14

18 years ago
Pavv, can you look at this one? On the Mac and Windows, we just added the 
resources and a couple of switch() cases... It's a 30 minutes work.

Comment 15

18 years ago
The CSS2 cursors seem to work well (tested at
http://www.people.fas.harvard.edu/~dbaron/css/test/sec1801), but in the third
case (cursor=auto in <a href="">) the cursor is a I-beam instead of a pointing
hand. (2000-08-13-20/Linux). (I haven't tested uri('*.cur').)
Keywords: mozilla1.0
Netscape's standard compliance QA team reorganised itself once again, so taking 
remaining non-tables style bugs. Sorry about the spam. I tried to get this done 
directly at the database level, but apparently that is "not easy because of the 
shadow db", "plus it screws up the audit trail", so no can do...
QA Contact: chrisd → ian

Comment 17

18 years ago
The problem reported by Tobias last year about "cursor:auto" is addressed in bug 
30971.  If that was the only problem left, maybe you can close this bug now.

Comment 18

18 years ago
Created attachment 40674 [details] [diff] [review]
patch for widget/src/gtk to implement most (except 3) cursors

Comment 19

18 years ago
Created attachment 40675 [details] [diff] [review]
widget/src/gtk/nsGtkCursor.h (part of patch)

Comment 20

18 years ago
Created attachment 40678 [details] [diff] [review]
widget/src/gtk/nsGtkCursorData.h (part of patch)

Comment 21

18 years ago
Created attachment 40679 [details] [diff] [review]
testcase to demonstrate new cursors

Comment 22

18 years ago
Okay, this patch adds GtkCreateCursor() function which creates a GdkCursor from
bitmap data + mask.
First patch is a diff for widget/src/gtk, and 2 files following are header files
to be placed in the same directory.
One contains the cursor names + structure name another has the cursor bits.

3 unimplemented cursors are countup countdown and countupdown, but I have *no*
idea what they should look like.
This patch also improves "question-arrow" look to be a bit more user friendly.

looking for reviews

Comment 23

18 years ago
Created attachment 40732 [details] [diff] [review]
new GtkCursorData with nice Spinning cursor from bug 48839

Comment 24

18 years ago
How to review this bug:

1. http://bugzilla.mozilla.org/showattachment.cgi?attach_id=40674
2. http://bugzilla.mozilla.org/showattachment.cgi?attach_id=40675
3. http://bugzilla.mozilla.org/showattachment.cgi?attach_id=40732

1) is the only "code" change, the other 2 files are just cursor data and some
structs.

Looking for r/sr
Identical code (adapted for xlib) has been already reviewed at bug 88457

Comment 25

18 years ago
r=tor
Please use enum names other than:

'GTK_CURSOR_QUESTION_ARROW'

They are in the GTK name space but they are specific to Mozilla.  That will help
tell when you should be using GtkCreateCursor() instead of gdk_cursor_new().  

Also, please cache the resulting values for the cursors somewhere instead of
re-creating them every time.  It's expensive if you don't.

Comment 27

18 years ago
Created attachment 41717 [details] [diff] [review]
new patch with cursor cache enabled

Comment 28

18 years ago
Created attachment 41718 [details] [diff] [review]
widget/src/gtk/nsGtkCursors.h

Comment 29

18 years ago
To review this code:

a) http://bugzilla.mozilla.org/showattachment.cgi?attach_id=41717
b) http://bugzilla.mozilla.org/showattachment.cgi?attach_id=41718

(a) is the patch and (b) is the cursor data

Comment 30

18 years ago
Created attachment 41719 [details] [diff] [review]
removed incorrect line of code (new patch, use same cursor data)
Two things:

Change:

+// #define DEBUG_CURSORCACHE
+

To:

+#undef DEBUG_CURSORCACHE

and remove this:

+  static GdkCursor  *GtkCursorCache[eCursor_count_up_down];

it doesn't appear to be used.  Do that and you have an sr=blizzard

Comment 32

18 years ago
Created attachment 41750 [details] [diff] [review]
final patch from blizzard's comments

Comment 34

18 years ago
Created attachment 41792 [details] [diff] [review]
new patch, critical crash bug missed by review

Comment 35

18 years ago
to check-in

a) attachment 41792 [details] [diff] [review] - patch
b) attachment 41718 [details] [diff] [review] - new file - widget/src/gtk/nsGtkCursors.h

Comment 36

18 years ago
Created attachment 41802 [details] [diff] [review]
minor changes to nsGtkCursors.h

Comment 37

18 years ago
Created attachment 41855 [details] [diff] [review]
updated version of nsGtkCursors.h

Comment 38

18 years ago
Created attachment 41856 [details] [diff] [review]
updated version of nsGtkCursors.h

Comment 39

18 years ago
Sorry about the spam.

Final version of GTK cursor patch ready to check-in:

a) attachment 41792 [details] [diff] [review]
b) attachment 41856 [details] [diff] [review]

the reason I had to re-post the cursor data was because I copied wrong license
boilerplate (NPL instead of MPL)

Comment 40

18 years ago
I just checked in pocemit's fix; I think we're done. If not someone 
should provide a list of what's left.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 41

18 years ago
Created attachment 41885 [details] [diff] [review]
fix for NS_ASSERTION

Comment 42

16 years ago
verified 
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.