Closed
Bug 1356072
Opened 7 years ago
Closed 7 years ago
stylo: Support moz-prefixed cursor values
Categories
(Core :: CSS Parsing and Computation, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla55
| Tracking | Status | |
|---|---|---|
| firefox55 | --- | fixed |
People
(Reporter: xidorn, Assigned: tommykuo)
References
Details
Attachments
(1 file, 1 obsolete file)
There are several moz-prefixed values for cursor property we may need to support. It seems cursor is a bit different that the keyword definition is in style_traits rather than style, and I'm not completely sure what's the best way here.
|
||
Updated•7 years ago
|
Priority: -- → P2
|
Assignee | |
Updated•7 years ago
|
Status: NEW → ASSIGNED
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8863659 -
Flags: review?(xidorn+moz)
Attachment #8863660 -
Flags: review?(xidorn+moz)
|
|
Assignee | |
Comment 4•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8863659 [details] Bug 1356072 - Make stylo support moz-prefixed cursor values. https://reviewboard.mozilla.org/r/135454/#review138426 ::: servo/components/style_traits/cursor.rs:80 (Diff revision 1) > "zoom-in" => ZoomIn = 33, > "zoom-out" => ZoomOut = 34, > + "-moz-grab" => MozGrab = 35, > + "-moz-grabbing" => MozGrabbing = 36, > + "-moz-zoom-in" => MozZoomIn = 37, > + "-moz-zoom-out" => MozZoomOut = 38, I have tried to make these lines be gecko-specific. But `[cfg(feature=gecko)]` seems not work with macro.
|
Reporter | |
Comment 5•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8863659 [details] Bug 1356072 - Make stylo support moz-prefixed cursor values. https://reviewboard.mozilla.org/r/135454/#review138426 > I have tried to make these lines be gecko-specific. But `[cfg(feature=gecko)]` seems not work with macro. You can add `$(#[$attr:meta])*` in the macro to support this.
|
|
Reporter | |
Comment 6•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8863660 [details] Bug 1356072 - Update stylo-failures. https://reviewboard.mozilla.org/r/135456/#review138434
Attachment #8863660 -
Flags: review?(xidorn+moz) → review+
|
|
Reporter | |
Comment 7•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8863659 [details] Bug 1356072 - Make stylo support moz-prefixed cursor values. https://reviewboard.mozilla.org/r/135454/#review138712 Please try making the new values Gecko-only.
Attachment #8863659 -
Flags: review?(xidorn+moz)
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 10•7 years ago
|
||
Hi Xidorn, I tried to add `#[$attr: meta])*` to the macro (see the latest patch), but I always got the following compilation error:
```
0:06.52 Compiling style_traits v0.0.1 (file:///Users/kuoe0/Works/Mozilla/gecko-dev/servo/components/style_traits)
0:06.90 error: local ambiguity: multiple parsing options: built-in NTs expr ('css') or 1 other option.
0:06.90 --> /Users/kuoe0/Works/Mozilla/gecko-dev/servo/components/style_traits/cursor.rs:79:5
0:06.90 |
0:06.90 79 | #[cfg(feature = "gecko")]
0:06.90 | ^
0:06.90
0:06.94 error: Could not compile `style_traits`.
```
Do you know what the correct syntax is to make it work?Flags: needinfo?(xidorn+moz)
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Reporter | |
Comment 13•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8863659 [details] Bug 1356072 - Make stylo support moz-prefixed cursor values. https://reviewboard.mozilla.org/r/135454/#review139134 This looks good to me. Probably worth double-check with Manish or emilio whether they think this is fine. ::: servo/components/style/properties/gecko.mako.rs:3835 (Diff revision 3) > + #[cfg(feature = "gecko")] > + Cursor::MozGrab => structs::NS_STYLE_CURSOR_GRAB, > + #[cfg(feature = "gecko")] > + Cursor::MozGrabbing => structs::NS_STYLE_CURSOR_GRABBING, > + #[cfg(feature = "gecko")] > + Cursor::MozZoomIn => structs::NS_STYLE_CURSOR_ZOOM_IN, > + #[cfg(feature = "gecko")] > + Cursor::MozZoomOut => structs::NS_STYLE_CURSOR_ZOOM_OUT, Do you need `cfg(feature = "gecko")` in `gecko.mako.rs`?
Attachment #8863659 -
Flags: review?(xidorn+moz) → review+
|
|
Reporter | |
Comment 14•7 years ago
|
||
emilio, are you fine with the approach in the patch?
Flags: needinfo?(xidorn+moz) → needinfo?(emilio+bugs)
|
||
Comment 15•7 years ago
|
||
Looks fine to me. I've seen this pattern used elsewhere too, but I forget where...
Flags: needinfo?(emilio+bugs)
|
|
Assignee | |
Comment 16•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8863659 [details] Bug 1356072 - Make stylo support moz-prefixed cursor values. https://reviewboard.mozilla.org/r/135454/#review139134 > Do you need `cfg(feature = "gecko")` in `gecko.mako.rs`? No, we don't need it. I think gecko.mako.rs is already gecko only.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8863659 -
Attachment is obsolete: true
|
||
Comment 20•7 years ago
|
||
Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0bb4e2e0a488 Update stylo-failures. r=xidorn
|
||
Comment 21•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/0bb4e2e0a488
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•