Closed Bug 1022084 Opened 8 years ago Closed 8 years ago

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

Categories

(DevTools Graveyard :: WebIDE, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: paul, Assigned: bgrins)

References

Details

Attachments

(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:  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)
(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: 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+
Just moved setupAutoCompletion() ouside of appended.
Attachment #8438492 - Attachment is obsolete: true
Attachment #8438648 - Flags: review+
Keywords: checkin-needed
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/e8cf4d699fa7
Status: ASSIGNED → RESOLVED
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.