Last Comment Bug 719028 - Style Editor does not highlight a few CSS2.0 and CSS3 properties
: Style Editor does not highlight a few CSS2.0 and CSS3 properties
Status: RESOLVED FIXED
[sourceeditor][orion][qa+]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: 11 Branch
: All All
: P3 normal (vote)
: Firefox 12
Assigned To: Alex Lakatos[:AlexLakatos]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-18 07:11 PST by Alex Lakatos[:AlexLakatos]
Modified: 2012-04-03 04:52 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
patch v1.0 (6.25 KB, patch)
2012-01-21 15:15 PST, Alex Lakatos[:AlexLakatos]
mihai.sucan: review+
Details | Diff | Splinter Review
patch v1.1 (6.26 KB, patch)
2012-01-23 08:43 PST, Alex Lakatos[:AlexLakatos]
mihai.sucan: review+
Details | Diff | Splinter Review

Description Alex Lakatos[:AlexLakatos] 2012-01-18 07:11:49 PST
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] 2012-01-18 08:22:17 PST
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] 2012-01-21 15:15:22 PST
Created attachment 590517 [details] [diff] [review]
patch v1.0

Added the 6 properties to CSS_KEYWORDS
Comment 3 Mihai Sucan [:msucan] 2012-01-23 08:29:51 PST
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] 2012-01-23 08:32:27 PST
(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] 2012-01-23 08:42:45 PST
(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] 2012-01-23 08:43:51 PST
Created attachment 590728 [details] [diff] [review]
patch v1.1

Addressed the nits
Comment 7 Mihai Sucan [:msucan] 2012-01-23 09:03:47 PST
Comment on attachment 590728 [details] [diff] [review]
patch v1.1

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

Thanks!
Comment 8 Paul Rouget [:paul] 2012-01-25 05:18:56 PST
https://hg.mozilla.org/integration/fx-team/rev/db26afebdd51
Comment 9 Tim Taubert [:ttaubert] 2012-01-25 09:03:07 PST
https://hg.mozilla.org/mozilla-central/rev/db26afebdd51
Comment 10 Simona B [:simonab ] 2012-04-03 04:52:50 PDT
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/.

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