Markup view "body #" search show suggestion "body #"

RESOLVED FIXED in Firefox 49

Status

()

Firefox
Developer Tools: Inspector
P3
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

47 Branch
Firefox 49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
STR:
1. Open `data:text/html;charset=utf-8,%0A<body>%0A<div id%3D"test">test<%2Fdiv>%0A<div>test<%2Fdiv>%0A<%2Fbody>%0A`
2. Open inspector
3. In the search field, type `body #`

Expected Result:
One suggestion ("body #test" is shown in the popup)

Actual Result:
There are two suggestion in the popup, "body #test", which is right, "body #", which seems wrong (pressing enter outputs "no results")


The problem comes from actor's `getSuggestionsForQuery` function (https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/inspector.js#2245). The sent `query` parameter is "body *", and thus `_multiFrameQuerySelectorAll`returns the 2 div child nodes, and then put their id in a set. But because only one of the two divs got an id, we end up having the "body #" suggestion.
A simple solution would be to test if the id of the node is not undefined before adding it to the result ( https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/inspector.js#2251 ).

However there is one test that rely on such suggestion ( https://dxr.mozilla.org/mozilla-central/source/devtools/client/inspector/test/browser_inspector_search-reserved.js#93 ), so I'm wondering if this is an expected behavior.
(Assignee)

Updated

2 years ago
Assignee: nobody → chevobbe.nicolas
Has STR: --- → yes
Priority: -- → P3
(Assignee)

Comment 1

2 years ago
Created attachment 8747333 [details]
MozReview Request: Bug 1269034 - Fix getSuggestionForQuery in inspector actor for id queries. r=bgrins

Make sure that result node id is not empty before appending it to the results

Review commit: https://reviewboard.mozilla.org/r/49843/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/49843/
Attachment #8747333 - Flags: review?(bgrinstead)
Comment on attachment 8747333 [details]
MozReview Request: Bug 1269034 - Fix getSuggestionForQuery in inspector actor for id queries. r=bgrins

https://reviewboard.mozilla.org/r/49843/#review46561

Works for me with a green try push, thanks
Attachment #8747333 - Flags: review?(bgrinstead) → review+
(Assignee)

Comment 3

2 years ago
TRY is mostly green ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=1da48d9dbb2f ), failed jobs have been successfully re-triggered.
Keywords: checkin-needed

Comment 4

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/1b930c1b6234
Keywords: checkin-needed

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1b930c1b6234
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
I have successfully reproduced this bug with Nightly 49.0a1 (2016-04-29) on windows 7 , 64 bit!

This bug's fix is verified on latest Developer Edition, Nightly.
Build  ID     20160708004052
User   Agent  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0


Build  ID     20160714030208
User  Agent   Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0

But I could not find the fix in beta version (48)
You need to log in before you can comment on or make changes to this bug.