1152721 - Move shared helper functions e.g. createChild() into a shared file

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

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

From a review comment by bgrins:

> ::: browser/devtools/styleinspector/computed-view.js
> @@ +1540,5 @@
> > + *        The tag name.
> > + * @param {object} aAttributes
> > + *        A set of attributes to set on the node.
> > + */
> > +function createChild(aParent, aTag, aAttributes={}) {
> 
> We should export this from a shared module since it's copy/pasted from the
> rule view.  Surprisingly, I don't think we have a shared file for this type
> of thing yet.  Maybe viewhelpers.jsm, or styleeditor/utils.js.  We can do
> that in a follow up though
>

Comment 1

[Julian Descottes]

Assigning this to me as follow up to Bug 1164343.

Comment 2

[Julian Descottes]

Attachment: Bug1152721.v1.patch

This patch moves helper methods from rule-view and computed-view to a new styleinspector/utils.js

I left some helper methods related to rule-view#getNodeInfo, because they are too specific to rule-view to be shared at the moment. 

Also removed the custom setTimeout clearTimeout to rely on timer.jsm instead.

Try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=55810880ce57

Comment 3

[Julian Descottes]

Thanks for the review ! 

Will rebase, do a new try push, and add checkin-needed if everything is fine.

Comment 4

[Julian Descottes]

Attachment: Bug1152721.v2.patch

New patch after rebase. No changes otherwise. 

New try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=72b8f28f54f8

Comment 5

[Julian Descottes]

Try is green, adding checkin-needed.

Comment 6

[Pulsebot]

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

Comment 7

[Carsten Book [:Tomcat]]

sorry had to backout this change for causing xpcshell bustage https://treeherder.mozilla.org/logviewer.html#?job_id=4072767&repo=fx-team

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/4d3ef8a98608

Comment 9

[Julian Descottes]

Sorry, my mistake.

I thought unit tests would be executed by my try push, but looks like I got the syntax wrong.

Comment 10

[Julian Descottes]

Attachment: Bug1152721.v3.patch

New push try, this time including xpcshell tests : https://treeherder.mozilla.org/#/jobs?repo=try&revision=f378fe7ac5cb

@Mike : since the previous patch was busted I prefer to get your approval on this one before going forward.d

Comment 11

[Julian Descottes]

Attachment: Bug1152721.v4.patch

Same as v3, just rebased. 

Try push (incl. xpcshell tests) : https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7d8d520cd86

Will add checkin-needed if try is green.

Comment 12

[Julian Descottes]

Except for unrelated issues, try is green.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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