137297 - Alphabetize queryableProperties

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

I've been running with this patch for a few days now and although it seemed
strange at first, I think it is on the whole easier to find properties in the
Computed Style view of Inspector if they are alphabetized.

After some thought, and actually running with the patch, I think the benefits of
alphabetizing the list outweigh the benefit of keeping related styles together.
 Currently, you have to know which styles are related, and then remember
approximately what position that group is in.  Alphabetizing is a clear way to
organize them and it is easy to find them right away.  Also, as we add more
computed style implementations, I think the benefit increases as we have less
'style soup' to wade through.

Additionally, people might expect these properties alphabetized if they use the
CSS Sidebar or Bradbury's TopStyle.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: v1.0

a, b, c, d, e, f, g....

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 79071 [details] [diff] [review]
v1.0

hmm, it seems I have font-weight and font-variant switched in this patch.  They
are fixed locally.

Comment 3

[Boris Zbarsky [:bzbarsky]]

You also have border_bottom_width and border_bottom_style switched.

And the margin_* properties are out of order (do we want to keep them that way,
since that's the order they come in in the shorthand?).  Same for padding.

eCSSProperty_outline lives in two places now, as do outline_*.  Not sure what
you want to do here.... thoughts?

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Heh, I looked over those properties a bunch of times and never noticed the top,
right, bottom, left mixup.  I'm just so used to seeing those in that order.  I
can see the benefit of them in the shorthand order, but I think that we should
alphabetize those too.

I intentionally left the outline stuff in two different places.  One is a
placeholder for the CSS2 outline* properties, and the other is for
-moz-outline*.  I know that gerv and dbaron are working on renaming some of
these things to have moz prefixes.  But I'm not sure if it is just for publicly
available names, or if it includes internal names as well.

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: v1.1

...h, i, j, k, l, m, n, o, p...

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 79075 [details] [diff] [review]
v1.1

r=bzbarsky

Comment 7

[Joe Hewitt (gone)]

Comment on attachment 79075 [details] [diff] [review]
v1.1

Could you put a comment at the top somewhere indicating that this list is
alphabetical, to remind people to keep it that way if they add new stuff?

I would have also liked to just sort the tree in the Computed Style view using
the sort service, so we didn't have to depend on the hard-coded order of this
array, but this will do for now.

sr=hewitt

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 79075 [details] [diff] [review]
v1.1

Checked in on trunk with a note about the list being alphabetized as per
hewitt, and also added at the start of the CSS2 properties to complement the
comment at the start of the -moz- properties.

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 79075 [details] [diff] [review]
v1.1

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 10

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in on the branch.  ->Fixed.

Comment 11

[Asa Dotzler [:asa]]

adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it
was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify
the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword
with verified1.0.0.