Bug 125235 - [CSS Shapes] Remove deprecated shapes
Summary: [CSS Shapes] Remove deprecated shapes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bem Jones-Bey
URL:
Keywords: InRadar
Depends on: 129880 129881 129882 129883
Blocks: 124173
  Show dependency treegraph
 
Reported: 2013-12-04 11:20 PST by Zoltan Horvath
Modified: 2014-03-12 08:42 PDT (History)
18 users (show)

See Also:


Attachments
Patch (529.94 KB, patch)
2014-03-06 15:42 PST, Bem Jones-Bey
no flags Details | Formatted Diff | Diff
Patch for landing (530.48 KB, patch)
2014-03-12 07:11 PDT, Bem Jones-Bey
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Horvath 2013-12-04 11:20:20 PST
Shapes specification no longer contains inset-rectangle. The new inset parsing support has been added in r159968, so we can remove the deprecated inset-rectangle support. I'll post the patch soon.
Comment 1 Bem Jones-Bey 2013-12-04 12:51:16 PST
(In reply to comment #0)
> Shapes specification no longer contains inset-rectangle. The new inset parsing support has been added in r159968, so we can remove the deprecated inset-rectangle support. I'll post the patch soon.

We shouldn't remove that until we update the demos/blogs/etc. In which case, I'd argue that we should just have one bug to remove all of the deprecated shapes at that time. (Perhaps change this one into "remove deprecated shapes")
Comment 2 Hans Muller 2013-12-04 13:36:01 PST
(In reply to comment #1)
> (In reply to comment #0)
> > Shapes specification no longer contains inset-rectangle. The new inset parsing support has been added in r159968, so we can remove the deprecated inset-rectangle support. I'll post the patch soon.
> 
> We shouldn't remove that until we update the demos/blogs/etc. In which case, I'd argue that we should just have one bug to remove all of the deprecated shapes at that time. (Perhaps change this one into "remove deprecated shapes"

It may not be practical to remove all of the deprecated stuff in one giant patch, but I agree that attempting to remove the bandage in as close to one quick gesture as possible is a good idea.
Comment 3 Radar WebKit Bug Importer 2014-01-31 11:06:00 PST
<rdar://problem/15958594>
Comment 4 Jon Lee 2014-03-04 21:57:47 PST
What's the latest on this?
Comment 5 Bem Jones-Bey 2014-03-05 07:19:07 PST
(In reply to comment #4)
> What's the latest on this?

I plan to remove inset-rectangle and the rest of the deprecated shapes in the next two weeks.
Comment 6 Bem Jones-Bey 2014-03-06 15:05:25 PST
I'm changing this to cover removing all the deprecated shapes. A patch is forthcoming that removes the following:

rectangle
inset-rectangle

It also removes the old syntax for ellipse and circle that used a comma separated argument list.
Comment 7 Bem Jones-Bey 2014-03-06 15:42:40 PST
Created attachment 226053 [details]
Patch
Comment 8 Bem Jones-Bey 2014-03-06 15:44:13 PST
smfr, krit: I figure you guys will want to take a look at this, since it affects clip-path as well as shapes.
Comment 9 Bem Jones-Bey 2014-03-07 07:40:15 PST
Comment on attachment 226053 [details]
Patch

After sleeping on this and letting my exuberance wear off, I will split this up into smaller patches; I'll do one for each shape type.

However, the rectangle patch is probably still going to be pretty huge on account of the number of tests that still use it.

This patch still shows what things will look like when the individual patches land.
Comment 10 Dirk Schulze 2014-03-07 08:14:58 PST
(In reply to comment #9)
> (From update of attachment 226053 [details])
> After sleeping on this and letting my exuberance wear off, I will split this up into smaller patches; I'll do one for each shape type.
> 
> However, the rectangle patch is probably still going to be pretty huge on account of the number of tests that still use it.
> 
> This patch still shows what things will look like when the individual patches land.

IIRC you are just removing code. So I don't think that this needs to be split. I just want to hear the opinion of smfr first.
Comment 11 Bem Jones-Bey 2014-03-07 09:24:28 PST
Comment on attachment 226053 [details]
Patch

Putting the r? back on Dirk's advice. I'll leave the dependent bugs for now waiting smfr's feedback.
Comment 12 Simon Fraser (smfr) 2014-03-07 10:19:58 PST
Did anyone every ship these? Not sure what went out in Safari 7/iOS 7.
Comment 13 Dirk Schulze 2014-03-07 10:31:02 PST
(In reply to comment #12)
> Did anyone every ship these? Not sure what went out in Safari 7/iOS 7.

We ship -webkit-clip-path and old syntax in iOS & Safari 7. Without inset-rect though.
Comment 14 Dean Jackson 2014-03-07 17:44:23 PST
(In reply to comment #13)
> (In reply to comment #12)
> > Did anyone every ship these? Not sure what went out in Safari 7/iOS 7.
> 
> We ship -webkit-clip-path and old syntax in iOS & Safari 7. Without inset-rect though.

So we're removing things that were shipping in iOS 7? Isn't this patch removing support for

-webkit-clip-path: rectangle(0%, 0%, 100%, 100%);
Comment 15 Dirk Schulze 2014-03-10 12:45:59 PDT
Comment on attachment 226053 [details]
Patch

Discussed this with Dean on #webkit. Yes, we are removing the support for the old syntax of circle(), ellipse() and rectangle() which would break content using these functions. The syntax for polygon() stays.

The patch looks good. Bem, please wait 24h to allow others to raise there concerns. If there are no objections, you can land the patch.
Comment 16 Bem Jones-Bey 2014-03-12 07:11:06 PDT
Created attachment 226501 [details]
Patch for landing
Comment 17 Bem Jones-Bey 2014-03-12 07:16:14 PDT
Comment on attachment 226501 [details]
Patch for landing

No one yelled, so sending to the CQ!
Comment 18 Bem Jones-Bey 2014-03-12 08:42:31 PDT
Committed r165472: <http://trac.webkit.org/changeset/165472>