Project Editor: turn on auto completion for JS & CSS files

RESOLVED FIXED in Firefox 33

Status

RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: paul, Assigned: bgrins)

Tracking

Trunk
Firefox 33

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

5 years ago
Created attachment 8437175 [details] [diff] [review]
projecteditor-autocomplete.patch

Heather, I also made a change to StyleSheetEditor.jsm when I realized there was an extra autocomplete related import while looking into how to do this.  I can break that out into a separate patch if you prefer.

Pushed to try:  https://tbpl.mozilla.org/?tree=Try&rev=618842328384
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attachment #8437175 - Flags: review?(fayearthur)
That try run looks pretty orange with some leaks. Known, or related?
Flags: needinfo?(bgrinstead)
(Assignee)

Comment 3

5 years ago
(In reply to Heather Arthur [:harth] from comment #2)
> That try run looks pretty orange with some leaks. Known, or related?

Not known, I'll have to look into this
Flags: needinfo?(bgrinstead)
(Assignee)

Updated

5 years ago
Attachment #8437175 - Flags: review?(fayearthur)
(Assignee)

Comment 4

5 years ago
Created attachment 8438492 [details] [diff] [review]
projecteditor-autocomplete.patch

I had to do some refactoring - it turns out that editor instances were not being properly destroyed if they were deleted.

* Moved resource deletion into the FilreResource intance instead of the tree
* Moved shell/editor destroying into the same place, and make sure to call it not just on page unload, but also when a resource is deleted
* Use shells.destroy() on page unload instead of looping through each resource and looking for an editor.  This simplifies the destroy function and also makes sure all shells are cleaned up.

This fixes the leaks locally - pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=c0c2f4f5ea70
Attachment #8437175 - Attachment is obsolete: true
Attachment #8438492 - Flags: review?(fayearthur)
Comment on attachment 8438492 [details] [diff] [review]
projecteditor-autocomplete.patch

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

Looks good.

::: browser/devtools/projecteditor/lib/editors.js
@@ +154,5 @@
>      });
>  
> +    this.appended = this.editor.appendTo(this.elt).then(() => {
> +      if (this.editor) {
> +        this.editor.setupAutoCompletion();

Not sure, but maybe appended doesn't have to wait for autocomplete:

```
this.appended = this.editor.appendTo(this.elt);
this.appended.then(() => { setupAutoCompletion()});
```
Attachment #8438492 - Flags: review?(fayearthur) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 8438648 [details] [diff] [review]
projecteditor-autocomplete-r=harth.patch

Just moved setupAutoCompletion() ouside of appended.
Attachment #8438492 - Attachment is obsolete: true
Attachment #8438648 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Hardware: x86_64 → All
https://hg.mozilla.org/integration/fx-team/rev/e8cf4d699fa7
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/e8cf4d699fa7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Blocks: 1030951
No longer blocks: 1030951

Updated

5 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.