Closed
Bug 1453258
Opened 6 years ago
Closed 6 years ago
calc should be supported in cursor
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: ericwilligers, Assigned: xidorn)
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36 Steps to reproduce: Set cursor inline style to 'url("https://example.com/") 1 calc(2 + 0), copy' Read cursor inline style and computed style Example page: https://jsfiddle.net/ericwilligers/ueeb1ene/ Actual results: Inline style is empty string. Computed style is 'auto' Expected results: 'url("https://example.com/") 1 2, copy' for both inline style and computed style.
Mozilla/5.0 (X11; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0 Build ID:20180416100103 Managed to reproduce the issue on Ubuntu 16.04 x64 and Windows 10 x64 using the latest Nightly 61.0a1 (2018-04-16).
Status: UNCONFIRMED → NEW
status-firefox61:
--- → affected
Component: Untriaged → CSS Parsing and Computation
Ever confirmed: true
Product: Firefox → Core
Comment 2•6 years ago
|
||
Heh, I knew this was familiar: https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/servo/components/style/values/computed/pointing.rs#115
Comment 3•6 years ago
|
||
(In reply to Eric Willigers from comment #0) > Expected results: > > 'url("https://example.com/") 1 2, copy' > for both inline style and computed style. This is not the expected result though, per spec we should preserve calc() in the specified value, so should be calc(1) calc(2) in the specified style.
Comment hidden (mozreview-request) |
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8968475 [details] Bug 1453258 - Support calc in cursor. https://reviewboard.mozilla.org/r/237178/#review242982 Nice cleanup :-). Are there any tests for this? If so, r=me. ::: servo/components/style/values/generics/pointing.rs:27 (Diff revision 1) > +/// > +/// https://drafts.csswg.org/css-ui/#cursor > +#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue)] > +pub struct Cursor<Image> { > + /// The parsed images for the cursor. > + pub images: Box<[Image]>, Doesn't `#[css(iterable, comma)]` allow you to derive this? ::: servo/components/style/values/generics/pointing.rs:71 (Diff revision 1) > + fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result > + where > + W: Write, > + { > + self.url.to_css(dest)?; > + if let Some((ref x, ref y)) = self.hotspot { Doesn't this do the same that the derived implementation would do? If so, just derive it. ::: servo/components/style/values/specified/pointing.rs:36 (Diff revision 1) > +} > + > +/// A specified value for the `cursor` property. > +pub type Cursor = generics::Cursor<CursorImage>; > + > +/// A specified value for the `image cursors`. This comment doesn't make much sense, mind rewording it? ::: servo/components/style/values/specified/pointing.rs:74 (Diff revision 1) > + }) > + } > +} > + > +impl CursorImage { > + fn parse_image<'i, 't>( Just `impl Parse for CursorImage`?
Attachment #8968475 -
Flags: review?(emilio) → review+
Assignee | ||
Comment 6•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8968475 [details] Bug 1453258 - Support calc in cursor. https://reviewboard.mozilla.org/r/237178/#review242982 > Doesn't `#[css(iterable, comma)]` allow you to derive this? Not really, because you need a comma between the last image and the keyword. I suppose using `ToCss` would generate a whitespace there. > Doesn't this do the same that the derived implementation would do? If so, just derive it. the trait `style_traits::ToCss` is not implemented for `(Number, Number)`.
Comment hidden (mozreview-request) |
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b00ed74f557b Support calc in cursor. r=emilio
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b00ed74f557b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•6 years ago
|
Assignee: nobody → xidorn+moz
Updated•6 years ago
|
QA Whiteboard: [good first verify]
You need to log in
before you can comment on or make changes to this bug.
Description
•