Closed Bug 1354000 Opened 7 years ago Closed 7 years ago

stylo: stylo doesn't understand moz-prefixed -moz-crisp-edges

Categories

(Core :: CSS Parsing and Computation, enhancement, P1)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
x86_64
Linux
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: shinglyu, Assigned: kuoe0.tw)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Affected test cases
9.21 KB, text/plain
Details
Bug 1354000 - Update test result.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
Stylo accepts "image-rendering:crisp-edges", not "image-rendering:-moz-crisp-edges". But gecko (and many testcases) still use -moz-crisp-edges. 

Should we:
1. Add both to the reftests?
2. Ship "crisp-edges" in Firefox?
3. Make Servo/Stylo understand "-moz-crisp-edges"?
Attached file Affected test cases — 

    
  
Assignee: nobody → kuoe0

    
    

    
  
(In reply to Shing Lyu [:shinglyu] from comment #0)
> Stylo accepts "image-rendering:crisp-edges", not
> "image-rendering:-moz-crisp-edges". But gecko (and many testcases) still use
> -moz-crisp-edges. 

Nice find! That looks like a lot of tests. Looks like Tommy is going to work on it?

> 
> Should we:
> 1. Add both to the reftests?
> 2. Ship "crisp-edges" in Firefox?
> 3. Make Servo/Stylo understand "-moz-crisp-edges"?

In general, we should do option (3). Changing the prefixes that Firefox ships is out of scope for stylo. I think this is probably a one-line annotation in the relevant .mako.rs file to enable the prefix on gecko builds.

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
      
  

    
    

    
      
  

    
  

        Attachment #8855664 -
        Flags: review?(cam)

        Attachment #8855665 -
        Flags: review?(cam)
Summary: stylo: stylo doesn't understand moz-prefixed -moz-crips-edges → stylo: stylo doesn't understand moz-prefixed -moz-crisp-edges

    
    

    
  
Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

https://reviewboard.mozilla.org/r/127520/#review130260

        Attachment #8855664 -
        Flags: review?(cam) → review+

    
    

    
  
Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

https://reviewboard.mozilla.org/r/127522/#review130262

Nice!

        Attachment #8855665 -
        Flags: review?(cam) → review+
Priority: -- → P1

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Hi Cameron, can you take a look the newer patch? I made some changes on it.
Flags: needinfo?(cam)

    
    

    
  
Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

https://reviewboard.mozilla.org/r/127520/#review131368

::: servo/components/style/properties/longhand/inherited_box.mako.rs:83
(Diff revisions 2 - 5)
> -                         "auto crisp-edges",
> +                         "auto",
>                           extra_gecko_values="optimizespeed optimizequality -moz-crisp-edges",
> -                         extra_servo_values="pixelated",
> +                         extra_servo_values="pixelated crisp-edges",
>                           custom_consts=image_rendering_custom_consts,
> -                         animatable=False,
> +                         animation_type="none",
> +                         needs_conversion="True",

Did you find this one was needed?  If not, please remove it.  The comment at http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/servo/components/style/properties/helpers.mako.rs#401 (this from_gecko_keyword function is generated when needs_conversion="True") seems like it's only intended for properties set by presentation attributes.

Otherwise, the patch looks good.
Flags: needinfo?(cam)

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c3e3315e68fb
(Part 1) Make stylo recognize -moz-crisp-edges keyword. r=heycam
https://hg.mozilla.org/integration/autoland/rev/979d89d569f2
(Part 2) Update test result. r=heycam
Keywords: checkin-needed

    
    

    
  
Oh, I forgot to split them... Thx!
Flags: needinfo?(kuoe0)

    
    

    
      
  

    
  

        Attachment #8855664 -
        Attachment is obsolete: true

    
  
Keywords: checkin-needed

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/4c23ea4277b6
Status: NEW → RESOLVED
Closed: 7 years ago

          status-firefox55:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

    
  
See Also:  → 1496617

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: