740603 - "Copy Rule" in the rule view includes expanded shorthand properties

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Dave Camp (:dcamp)]

* Point the inspector at an element with a shorthand property
* Use "Copy Rule" from the context menu
* Paste the resulting text

All the expanded shorthand properties are included.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Made "Copy Rule" more robust

Seems like this must be an OSX issue only as it works fine on Linux & Windows.

I have made the copy rule & declaration methods a little more robust so it should hopefully fix the problem.

Comment 2

[Dave Camp (:dcamp)]

Comment on attachment 610843 [details] [diff] [review]
Made "Copy Rule" more robust

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

Needs tests for the things that were failing.

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Added test

Dave, I am asking for feedback from you as discussed.

See:
https://tbpl.mozilla.org/?tree=Try&rev=1c403040a6db

There are timing issues relating to the rule view ... it seems like the DOM is not fully initialized at the time the rule view sends out it's ready notifier.

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 612371 [details] [diff] [review]
Added test

OK, between synthesizing the mouse click on the property name and working on its editor, you need to wait for the editor to be created.  The browser_ruleview_ui.js test has a method that will wait for an editor to be created, you should be able to use something like that.

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch now includes wait for editor focus and blur

This was held up by a hard to find leak (fixed by somebody else somewhere).

Patch works fine locally but need to wait for try:
https://tbpl.mozilla.org/?tree=Try&rev=0f011861fc57

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

dcamp: Seems like my latest patch is already attached

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Now green on try

Fixed oranges and timing issues. If we can get this into Aurora then we would not need to disable the copy menus.

Comment 8

[Dave Camp (:dcamp)]

Comment on attachment 614163 [details] [diff] [review]
Now green on try

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

::: browser/devtools/styleinspector/CssRuleView.jsm
@@ +1021,1 @@
>  

Is it ever possible for node to be null here?  If so, please protect against that.

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Addressed reviewers comments

(In reply to Dave Camp (:dcamp) from comment #8)
> Comment on attachment 614163 [details] [diff] [review]
> Now green on try
> 
> Review of attachment 614163 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/styleinspector/CssRuleView.jsm
> @@ +1021,1 @@
> >  
> 
> Is it ever possible for node to be null here?  If so, please protect against
> that.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 11

[Rob Campbell [:rc] (:robcee)]

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