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

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Developer Tools
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dcamp, Assigned: dcamp)

Tracking

10 Branch
Firefox 10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 572135 [details] [diff] [review]
WIP 1

... to let the highlighter and other tools know to update themselves after a change.
Attachment #572135 - Flags: feedback?(rcampbell)
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).
Attachment #572135 - Flags: feedback?(rcampbell) → feedback+
(Assignee)

Comment 2

6 years ago
* 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.
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.
Status: NEW → ASSIGNED
(Assignee)

Comment 4

6 years ago
Created attachment 572242 [details] [diff] [review]
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.
Assignee: nobody → dcamp
Attachment #572135 - Attachment is obsolete: true
Attachment #572242 - Flags: review?(rcampbell)
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!
Attachment #572242 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 6

6 years ago
(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.
(Assignee)

Comment 7

6 years ago
Created attachment 572376 [details] [diff] [review]
v2

Fixed review comments and a minor test mistake.
Attachment #572242 - Attachment is obsolete: true
(Assignee)

Comment 8

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/815234795765
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/815234795765
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
You need to log in before you can comment on or make changes to this bug.