Closed Bug 1365910 Opened 7 years ago Closed 7 years ago

Clear icon of textbox[type="search"] should match the spec

Categories

(Firefox :: Settings UI, defect, P1)

Product:
Firefox
Component:
Settings UI
Version:
55 Branch
Platform:
Unspecified
Linux
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- affected

People

(Reporter: hyacoub, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

X.png
231.20 KB, image/png
Details
Bug 1365910 - Clear icon of textbox[type="search"] should match the spec
59 bytes, text/x-review-board-request
dao
: review-
Details
Attached image X.png — 
[Affected versions]: 
Nightly 55.0a1

[Affected platforms]:
Platforms: Ubuntu 16.10

[Steps to reproduce]:
1. Launch Firefox, go to about:config and search for "browser.preferences.search" and set it value to true.
2. Go to "about:preferences".
3. Type in the preferences search field.

[Expected result]:
"X" button in the preferences search field is correctly displayed.

[Actual result]:
"X" button in the preferences search field isn't correctly displayed.
Blocks: 1357285
No longer blocks: 1357306
Hani, could you help to check this issue also happen on Linux and macOS? Thanks
Flags: needinfo?(hani.yacoub)
I could only reproduce it on Ubuntu 16.04 x64.
Flags: needinfo?(hani.yacoub)
Whiteboard: [photon-preference]
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Attachment #8869903 - Flags: review?(jaws) → review?(dao+bmo)
Comment on attachment 8869903 [details]
Bug 1365910 - Clear icon of textbox[type="search"] should match the spec

https://reviewboard.mozilla.org/r/141450/#review145222

::: toolkit/content/textbox.css:24
(Diff revision 1)
>    box-sizing: border-box;
>    -moz-box-flex: 1;
>  }
>  
> +.textbox-search-clear {
> +  list-style-image: url(chrome://global/skin/icons/searchfield-cancel.svg);

This belongs in themes rather than content.

Having said that, the clear icon isn't broken on Linux. It's just different as it's a stock icon provided by the OS.
Attachment #8869903 - Flags: review?(dao+bmo) → review-
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
Sorry I wasn't clear on my comment. This feature belongs to Photon preferences which applies new look and feel of new clear icon in search field, so it still makes sense to replace the searchfield-cancel.svg to all platforms.

The design spec of search field: https://mozilla.invisionapp.com/share/ZDAGPK3AF#/screens/218928188
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Tina, could you confirm this clear icon in search field should take the same look and feel cross platforms?
Flags: needinfo?(thsieh)
Summary: "X" button in the preferences search field isn't correctly displayed → Clear icon of textbox[type="search"] should match the spec
Per our discussion, we will use the clear icon for Mac OS as default and reuse it on different OS for now.
Let me know if any questions. Thanks!
Flags: needinfo?(thsieh)
Dao, as visual confirmation on comment 7, we're going to apply the current macOS cancel icon across all platforms. So I think it's reasonable to move searchfield-cancel.svg from osx folder to shared.

Could you help review the patch? thanks!
(In reply to Helen Huang [:HHuang] from comment #7)
> Per our discussion, we will use the clear icon for Mac OS as default and
> reuse it on different OS for now.
> Let me know if any questions. Thanks!

First question would be: Why? What problem are you trying to solve? The backspace icon seems to be used consistently in search fields on Linux.
Flags: needinfo?(rchien)
Comment on attachment 8869903 [details]
Bug 1365910 - Clear icon of textbox[type="search"] should match the spec

https://reviewboard.mozilla.org/r/141450/#review145434

::: toolkit/content/textbox.css:24
(Diff revision 2)
>    box-sizing: border-box;
>    -moz-box-flex: 1;
>  }
>  
> +.textbox-search-clear {
> +  list-style-image: url(chrome://global/skin/icons/searchfield-cancel.svg);

As mentioned earlier, this doesn't belong in toolkit/content/.

searchfield-cancel.svg also was made to fit Mac's round search fields. On Windows and Linux these fields are sqare, so this doesn't really make sense.
Attachment #8869903 - Flags: review?(dao+bmo) → review-
Status: REOPENED → ASSIGNED
Target Milestone: --- → Firefox 55
Comment on attachment 8869903 [details]
Bug 1365910 - Clear icon of textbox[type="search"] should match the spec

https://reviewboard.mozilla.org/r/141450/#review146834

::: toolkit/themes/shared/in-content/common.inc.css:481
(Diff revision 6)
>  xul|textbox[type="search"] > .textbox-input-box > .textbox-search-icons > .textbox-search-icon {
>    visibility: hidden;
>  }
>  
> +xul|textbox[type="search"] > .textbox-input-box > .textbox-search-icons > .textbox-search-clear {
> +  list-style-image: url(chrome://global/skin/icons/searchfield-cancel.svg);

This is still an issue: "searchfield-cancel.svg was made to fit Mac's round search fields. On Windows and Linux these fields are sqare, so this doesn't really make sense."

Also, still no word on why we should do this (comment 10). As best I can tell, we should just resolve this as wontfix and move on. The amount of time we have already spent on this seems unwarranted.
Attachment #8869903 - Flags: review?(dao+bmo) → review-
As comment 5 mentioned, this is a feature and bug title has changed according to spec. As visual confirmation on comment 7 we've decided to reuse "searchfield-cancel.svg" for all platforms. This change will affect to in-content scope but not all toolkit widget.

As a result, search field's "x" button (textbox[type="search"] in about:preferences will have the same styling on all platforms.
Flags: needinfo?(rchien)
I'm not interested in a mere decision but in reasons why that's the right decision. You still haven't provided that.
(In reply to Dão Gottwald [::dao] from comment #10)
> (In reply to Helen Huang [:HHuang] from comment #7)
> > Per our discussion, we will use the clear icon for Mac OS as default and
> > reuse it on different OS for now.
> > Let me know if any questions. Thanks!
> 
> First question would be: Why? What problem are you trying to solve? The
> backspace icon seems to be used consistently in search fields on Linux.

(In reply to Dão Gottwald [::dao] from comment #11)
> searchfield-cancel.svg also was made to fit Mac's round search fields. On
> Windows and Linux these fields are sqare, so this doesn't really make sense.

Hi Helen, could you please answer the above questions?

I'm inclined to close this bug as wontfix so we can move on to more important things, unless there are compelling reasons why we should use the Mac icon across platforms.

Thanks.
Flags: needinfo?(hhuang)
Comment on attachment 8869903 [details]
Bug 1365910 - Clear icon of textbox[type="search"] should match the spec

https://reviewboard.mozilla.org/r/141450/#review148254

See my previous comments.
Attachment #8869903 - Flags: review?(dao+bmo) → review-
Ah, sorry I just rebase to fix conflict but doesn't mean I'd like to ask review. Review has been canceled.
To answer the question from Comment 19, using "searchfield-cancel.svg" is a temporary solution for now. Currently we are still building up the design system for Photon, so there is no a specific rule for when we should use different icon and when to use a unified icon for platforms. 
Therefore, I agree with close this bug for now, till we decide whether or not to use different icons for each platform.
Flags: needinfo?(hhuang)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → WONTFIX
Comment on attachment 8869903 [details]
Bug 1365910 - Clear icon of textbox[type="search"] should match the spec

https://reviewboard.mozilla.org/r/141450/#review148274
Attachment #8869903 - Flags: review?(dao+bmo) → review-
Whiteboard: [photon-preference]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: