Closed Bug 1400615 Opened 2 years ago Closed 2 years ago

Search field in Storage Inspector has filter and search icon

Categories

(DevTools :: Storage Inspector, defect, P3)

56 Branch
All
Windows 10
defect

Tracking

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 wontfix, firefox57 verified, firefox58 verified)

VERIFIED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- wontfix
firefox57 --- verified
firefox58 --- verified

People

(Reporter: sebo, Assigned: abhinav.koppula)

References

Details

(Keywords: polish, regression)

Attachments

(2 files)

The search field within the Storage Inspector now has a filter and a search icon. See the attached screenshot.

It should just have a filter icon.

According to mozregression this is a regression caused by bug 1380268.

Sebastian
Setting to P3 but would appreciate a confirmation from front end devs.
Flags: needinfo?(dao+bmo)
Keywords: polish
Priority: -- → P3
I'm not involved with Developer Tools: Storage Inspector and don't know what this bug is talking about.
Flags: needinfo?(dao+bmo) → needinfo?(mratcliffe)
Sebastian, I can't reproduce on macOS, can you provide a screenshot?

Thanks.
Flags: needinfo?(sebastianzartner)
Excuse the delay! Here's a screenshot including how the code looks like in the Browser Toolbox.

Sebastian
Flags: needinfo?(sebastianzartner)
Hardware: x86_64 → All
Version: 57 Branch → 56 Branch
Regression of photon-visual change
Whiteboard: [photon-visual][triage]
Thank you :sebo!

This needs an extra selector at: https://dxr.mozilla.org/mozilla-central/source/devtools/client/themes/common.css#445

for .textbox-search-sign
Devtools folks will need to take care of this.
Whiteboard: [photon-visual][triage]
Hi Sebastian, Tim,
I have tried fixing the issue by adding a `display:none` rule. Does this look fine?
Comment on attachment 8919609 [details]
Bug 1400615 - Hide search icon in the search field of Storage Inspector.

I'm very busy these days. Tim, could you please do the review?

Sebastian
Attachment #8919609 - Flags: review?(sebastianzartner) → review?(ntim.bugs)
Comment on attachment 8919609 [details]
Bug 1400615 - Hide search icon in the search field of Storage Inspector.

https://reviewboard.mozilla.org/r/190496/#review197402

::: devtools/client/themes/common.css:441
(Diff revision 1)
> +.devtools-filterinput > .textbox-input-box > .textbox-search-sign {
> +  display: none;
> +}
> +

:bgrins, can you check if visibility: hidden; is enough (like the rule below), or whether this needs display: none ?
Attachment #8919609 - Flags: review?(ntim.bugs)
Comment on attachment 8919609 [details]
Bug 1400615 - Hide search icon in the search field of Storage Inspector.

https://reviewboard.mozilla.org/r/190496/#review197404

Redirecting review to :bgrins who has a windows machine.
Attachment #8919609 - Flags: review?(bgrinstead)
Comment on attachment 8919609 [details]
Bug 1400615 - Hide search icon in the search field of Storage Inspector.

https://reviewboard.mozilla.org/r/190496/#review197408

This looks fine to me with one small nit: please add a comment above this rule explaining that it's needed to remove the default search icon on windows (with a link to this bug)

::: devtools/client/themes/common.css:441
(Diff revision 1)
> +.devtools-filterinput > .textbox-input-box > .textbox-search-sign {
> +  display: none;
> +}
> +

It needs to be `display: none`, otherwise it still takes space before the text in the input.
Attachment #8919609 - Flags: review?(bgrinstead) → review+
Hi Brian,
I have updated the mozreview-request with the comment.
Thanks
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/afde6e60e63d
Hide search icon in the search field of Storage Inspector. r=bgrins
Comment on attachment 8919609 [details]
Bug 1400615 - Hide search icon in the search field of Storage Inspector.

Approval Request Comment
[Feature/Bug causing the regression]: Photon visual change (bug 1380268)
[User impact if declined]: see attachment 8914277 [details], double filter and search icon on storage inspector and canvas debugger
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: local testing
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: small CSS change
[String changes made/needed]: no
Flags: needinfo?(mratcliffe)
Attachment #8919609 - Flags: approval-mozilla-beta?
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/afde6e60e63d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment on attachment 8919609 [details]
Bug 1400615 - Hide search icon in the search field of Storage Inspector.

Recent regression, low risk, Beta57+
Attachment #8919609 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [good first verify]
I have reproduced this Bug with Nightly 57.0a1 (2017-09-16) on Windows 7, 64 Bit!

This bug's fix is Verified with latest Beta & latest Nightly !

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

Build   ID    20171026221945
User Agent    Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
QA Whiteboard: [good first verify] → [good first verify] [testday-20171027]
I can also confirm that it's fixed now in Nightly 58.0a1 (2017-10-29) and Beta 57.0b12.
Thank you for the fix, Abhinav!

Sebastian
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.