719028 - Style Editor does not highlight a few CSS2.0 and CSS3 properties

Status: RESOLVED FIXED

(DevTools :: General, defect, P3)

Description

[Alex Lakatos[:AlexLakatos]]

Style editor does not highlight these properties yet:

CSS 3:
flex-align
flex-flow
flex-line-pack
flex-order
flex-pack

CSS 2.0:
marker-offset

The properties list was taken from http://meiert.com/en/indices/css-properties/ . Every other property listed there is highlighted.

Comment 1

[Paul Rouget [:paul]]

These need to be added here: http://mxr.mozilla.org/mozilla-central/source/browser/devtools/sourceeditor/orion/orion.js#10474

Comment 2

[Alex Lakatos[:AlexLakatos]]

Attachment: patch v1.0

Added the 6 properties to CSS_KEYWORDS

Comment 3

[Mihai Sucan [:msucan]]

Comment on attachment 590517 [details] [diff] [review]
patch v1.0

Review of attachment 590517 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for your contribution Alex! r+!

A couple of comments below...

::: browser/devtools/sourceeditor/orion/orion.js
@@ +9,5 @@
>   * Contributors: 
>   *		Felipe Heidrich (IBM Corporation) - initial API and implementation
>   *		Silenio Quarti (IBM Corporation) - initial API and implementation
>   *		Mihai Sucan (Mozilla Foundation) - fix for Bug#364214
> + *		Alex Lakatos (Mozilla Contributor) - fix for Bug#719028

s/Bug/Mozilla Bug/

This is a Mozilla bug number, not an Eclipse project bug number.

@@ +10492,5 @@
> +		 "drop-initial-value", "elevation", "empty-cells", "fit", "fit-position", "flex-align", "flex-flow", "flex-inline-pack", "flex-order",
> +		 "flex-pack", "float", "float-offset", "font", "font-family", "font-size", "font-size-adjust", "font-stretch", "font-style",
> +		 "font-variant", "font-weight", "grid-columns", "grid-rows", "hanging-punctuation", "height", "hyphenate-after",
> +		 "hyphenate-before", "hyphenate-character", "hyphenate-lines", "hyphenate-resource", "hyphens", "icon", "image-orientation",
> +		  "image-rendering", "image-resolution", "inline-box-align", "left", "letter-spacing", "line-height", "line-stacking",

Nit: this line has an additional space that you should remove.

Comment 4

[Alex Lakatos[:AlexLakatos]]

(In reply to Mihai Sucan [:msucan] from comment #3)
> > + *		Alex Lakatos (Mozilla Contributor) - fix for Bug#719028
> 
> s/Bug/Mozilla Bug/
> 
> This is a Mozilla bug number, not an Eclipse project bug number.
Should I log an Eclipse bug as well?

Comment 5

[Mihai Sucan [:msucan]]

(In reply to Alex Lakatos from comment #4)
> (In reply to Mihai Sucan [:msucan] from comment #3)
> > > + *		Alex Lakatos (Mozilla Contributor) - fix for Bug#719028
> > 
> > s/Bug/Mozilla Bug/
> > 
> > This is a Mozilla bug number, not an Eclipse project bug number.
> Should I log an Eclipse bug as well?

For the purpose of fixing this bug in our codebase, that's not needed. However, you are free to do so, if you want to make this contribution upstream.

I suggested you replace "fix for Bug#719028" with "fix for Mozilla Bug#719028". Thanks!

Comment 6

[Alex Lakatos[:AlexLakatos]]

Attachment: patch v1.1

Addressed the nits

Comment 7

[Mihai Sucan [:msucan]]

Comment on attachment 590728 [details] [diff] [review]
patch v1.1

Review of attachment 590728 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 8

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/db26afebdd51

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/db26afebdd51

Comment 10

[Simona Badau, Desktop QA]

The property flex-line-pack is not highlighted. Instead flex-inline-pack it is even though does not appear in the properties list here http://meiert.com/en/indices/css-properties/.