Closed Bug 1022084 Opened 8 years ago Closed 8 years ago

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


(DevTools Graveyard :: WebIDE, defect)

Not set


(Not tracked)

Firefox 33


(Reporter: paul, Assigned: bgrins)




(1 file, 2 obsolete files)

No description provided.
Attached patch projecteditor-autocomplete.patch (obsolete) — Splinter Review
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:
Assignee: nobody → bgrinstead
Attachment #8437175 - Flags: review?(fayearthur)
That try run looks pretty orange with some leaks. Known, or related?
Flags: needinfo?(bgrinstead)
(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)
Attachment #8437175 - Flags: review?(fayearthur)
Attached patch projecteditor-autocomplete.patch (obsolete) — Splinter Review
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:
Attachment #8437175 - Attachment is obsolete: true
Attachment #8438492 - Flags: review?(fayearthur)
Comment on attachment 8438492 [details] [diff] [review]

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+
Just moved setupAutoCompletion() ouside of appended.
Attachment #8438492 - Attachment is obsolete: true
Attachment #8438648 - Flags: review+
Keywords: checkin-needed
Hardware: x86_64 → All
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
No longer blocks: 1030951
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.