Closed Bug 879523 Opened 7 years ago Closed 7 years ago

Remote the SelectorSearch

Categories

(DevTools :: Inspector, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: dcamp, Assigned: dcamp)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch WIP (obsolete) — Splinter Review
This WIP doesn't do autocompletion yet.
Attached patch v1 (obsolete) — Splinter Review
I'll file a followup to port the autocompletion.
Assignee: nobody → dcamp
Attachment #758240 - Attachment is obsolete: true
Attachment #766736 - Flags: review?(paul)
Attachment #766736 - Flags: feedback?(scrapmachines)
Comment on attachment 766736 [details] [diff] [review]
v1

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

looks good. Although I am just assuming that the tests pass.

::: browser/devtools/inspector/selector-search.js
@@ +217,5 @@
> +    this._lastQuery = this.walker.querySelectorAll(this.walker.rootNode, query).then(list => {
> +      return list;
> +    }, (err) => {
> +      // Failures are ok here, just use a null item list;
> +      return null;l

too much nullness

@@ +230,5 @@
>  
> +      // Value has changed since we started this request, we're done.
> +      if (query != this.searchBox.value) {
> +        return;
> +      }

umm, again ?
Attachment #766736 - Flags: feedback?(scrapmachines) → feedback+
(In reply to Girish Sharma [:Optimizer] from comment #2)

> 
> @@ +230,5 @@
> >  
> > +      // Value has changed since we started this request, we're done.
> > +      if (query != this.searchBox.value) {
> > +        return;
> > +      }
> 
> umm, again ?

Yeah, because we do two async requests, so the searchbox might have changed in the meantime either time.
Attached patch v2Splinter Review
I take that back.  New version.
Attachment #766736 - Attachment is obsolete: true
Attachment #766736 - Flags: review?(paul)
Attachment #767388 - Flags: review?(paul)
Attachment #767388 - Flags: review?(paul) → review+
https://hg.mozilla.org/mozilla-central/rev/40fcf0ac7aba
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.