699978 - Tools should notify the highlighter when they've modified a node

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Dave Camp (:dcamp)]

Attachment: WIP 1

... to let the highlighter and other tools know to update themselves after a change.

Comment 1

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

Comment on attachment 572135 [details] [diff] [review]
WIP 1

looks good. We'll need to add the onUpdate references and methods to each of the registration objects and tools (I think the only one that'll really need it is the style inspector).

Comment 2

[Dave Camp (:dcamp)]

* Style inspector needs it (and should be easy, it already has refresh code)
* Rule View needs it (and will take a bit of code, in progress.  Will be needed if someone edits a style attribute in html view)
* HTML view needs it (when someone edits a style attribute in rule view).

Was planning to do each of these as followups, but we could do them all here.

Comment 3

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

your call, depending on the workload. Style inspector should be easy. just a matter of telling it to reselect the node.

Ruleview is baby, so you should know what needs doing.

HTML view is the trickiest of the bunch because of the ridiculous way the tree panel is constructed. Talked a bit about this in IRC, am willing to leave this as a followup.

Comment 4

[Dave Camp (:dcamp)]

Attachment: final v1

OK, this version makes sure the highlighter rectangle and the style inspector are updated on changes from the html panel or the rule view.

Comment 5

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

Comment on attachment 572242 [details] [diff] [review]
final v1

   /**
+   * Notify register tools of changes to the highlighted element.
+   * @param object aUpdater

probably should read "notify registered tools". That is a pretty nitty comment.

CssRuleView.jsm

+  /**
    * Creates editor UI for each of the rules in _elementStyle.
    */
   _createEditors: function CssRuleView_createEditors()
@@ -1003,10 +1034,12 @@ TextPropertyEditor.prototype = {
    */
   _parseValue: function TextPropertyEditor_parseValue(aValue)

should probably document aValue and @return

ok!

Comment 6

[Dave Camp (:dcamp)]

(In reply to Rob Campbell [:rc] (robcee) from comment #5)
> Comment on attachment 572242 [details] [diff] [review] [diff] [details] [review]
> final v1
> 
>    /**
> +   * Notify register tools of changes to the highlighted element.
> +   * @param object aUpdater
> 
> probably should read "notify registered tools". That is a pretty nitty
> comment.
> 

Oops, updated.

>     */
>    _parseValue: function TextPropertyEditor_parseValue(aValue)
> 
> should probably document aValue and @return

Bad context, those are documented before the diff cutoff.

Comment 7

[Dave Camp (:dcamp)]

Attachment: v2

Fixed review comments and a minor test mistake.

Comment 8

[Dave Camp (:dcamp)]

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

Comment 9

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

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