From Slide 9 of https://bugzilla.mozilla.org/attachment.cgi?id=8819509 There should be a magnifying glass in the search input before the placeholder. The attached SVG is the image they're quested we use. As of this comment, the spec says 14 x 14 at #9B9B9B

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review107180 Good start, though we should change directions a bit. Nice job on getting your first patch uploaded to bugzilla and requesting review correctly. ::: browser/components/preferences/cookies.xul:34 (Diff revision 1) > - <hbox align="center"> > + <hbox align="center" id="searchIconAndTextContainer"> > + <div id="searchIcon2"></div> This isn't the right route to take with changing how the search textbox gets changed. When I applied your patch and built it, I ended up seeing two magnifying glasses (the one that you added and the one that was already there). You should instead look at toolkit/content/widgets/textbox.xml, starting at line 313. This defines how textboxes that have type="search" should look and behave. Here's a permalink to the line that I'm referencing, http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/content/widgets/textbox.xml#313 ::: browser/themes/shared/incontentprefs/preferences.inc.css:197 (Diff revision 1) > padding: 15px 0; > } > > #filter { > margin-inline-start: 0; > + padding-left: 2.2em; We can't use properties like "padding-left" or "padding-right" because we need them to flip their direction if the browser is using a right-to-left locale such as Hebrew or Arabic. We instead need to use locale-independent properties, like the rule above is using. If you think you need to use padding-left, you should instead use padding-inline-start. This way it will look correct for left-to-right text as well as right-to-left text.

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review107304 Please remove the configure and configure-old files that you added. Also, it doesn't look like your new patch will use the new magnifying glass nor put it in the front. Am I wrong? I tried importing your patch but I ran in to issues. Please contact me through IRC and we can find a time to do a video chat so you can get moving on this bug. Thanks! :)

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review108004 This is the same patch that was uploaded earlier and I reviewed in https://bugzilla.mozilla.org/show_bug.cgi?id=1330115#c2 Please follow my feedback there before requesting review again. As always, I am available to answer questions.

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review108312 ::: browser/components/preferences/cookies.xul:36 (Diff revision 4) > <textbox type="search" id="filter" flex="1" > aria-controls="cookiesList" > oncommand="gCookiesWindow.filter();" > placeholder="&searchFilter.label;" > accesskey="&searchFilter.accesskey;"/> So hopefully by now, you've read up on Web Components, and have some sense of what they are and what the Shadow DOM is. Now let's bring it all back to XBL. XBL and "anonymous content" is basically the pre-cursor to Web Components and Shadow DOM. The way this works is, this element: ```XML <textbox type="search" ... /> ``` Has this CSS rule applied to it: http://searchfox.org/mozilla-central/source/toolkit/content/xul.css#792-794 And this line: http://searchfox.org/mozilla-central/source/toolkit/content/xul.css#793 Says, "apply the binding that you find at this URL: 'chrome://global/content/bindings/textbox.xml#search-textbox'", which is the file (and binding) that jaws pointed at in his comment. Once the styling mechanism sees that rule, the binding is applied to the <textbox>, and it assumes the behaviour of the binding. It also gets the bindings <content>: http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/content/widgets/textbox.xml#314-328 That <content> is inserted inside of the <textbox> as anonymous content, so that in some regards, it's invisible to outside consumers. The idea here is that common widgets, like "input boxes that do searching", can be shared, and be encapsulated. So if you look at the anonymous content that gets inserted into the <textbox>, one of those things is something called a <xul:deck>: http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/toolkit/content/widgets/textbox.xml#319-326 . A <xul:deck> is something that only shows one of its children at a time. You use JavaScript to tell it the index of the child to display. This is actually how Firefox tabs work - the things that show web content are a "deck", and the "tab strip" uses JavaScript to tell the deck which web content to display. [Here's more information on "deck"](https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/deck) So now we know that this <textbox> has a <deck> inside it, and that <deck> has the attribute anonid="searchicons". anonid isn't special - it's just a way of referring to anonymous content that is supposed to be unique within the scope of the anonymous content. We can't use id because id's need to be unique for the entire document. Anyhow, inside that deck, we can see that there are two images. One appears to be a "search" icon, and one is a "clear" icon. What this means is that this <textbox> has everything we need already. But why isn't it being displayed? At least for me on OS X? And why does a magnifying glass display on Windows? If you use the Firefox Developer Tools like I showed you last meeting, and try to inspect the <textbox>, you should be able to see the anonymous content, and should be able to inspect the styles applied to them. One thing you should notice (if you're on OS X) is that the <deck> has visibility set to hidden on it by default with this rule: ```css .textbox-search-icons:not([selectedIndex="1"]) { visibility: hidden; } ``` Note that there's some logic in the search-textbox binding that makes it so that if some input goes into the search field, the "selectedIndex" changes to 1, so the <deck> becomes visible. This is what powers the "X" that shows up when some text goes into the search field! So that's why there are 2 magnifying glasses on some platforms - on some platforms (like Windows), we already show a magnifying glass. Now that we know this - it raises another question: how should that magnifying glass behave when the user types something into the search field? Should it turn into an X like the current one works? Or is that magnifying glass supposed to stay there? That last question is one that I think we should forward to UX for more information. If we want to keep the magnifying glass around (like we do elsewhere - example, the about:newtab page), then we should probably _hide_ the the <deck> entirely with some CSS, to get rid of the magnifying glass and X icon. We should then add some more CSS to insert a magnifying glass before the text input, and shift the text input over to give it space. I'm guess that's likely the path that UX will suggest, so for now, let's go that route. I'll try to get a definitive answer from UX though in the bug.

Hey Tina, for the magnifying glass in the search input... We already show a magnifying glass for the Windows platform. It's on the right side, and it turns into an X to clear the search input if there's text placed inside of it. With this spec, should we be showing that same magnifying glass / clear icon to the _left_ of the text input (or rather, at the start of the text input, for RTL locales)? Or do we want to get rid of the clear icon logic entirely, and just place a magnifying glass at the start of the text input that stays there always, regardless of whether or not there's text in there (like in about:newtab)?

Hi Mike, The direction of the magnifying glass should follow the platform design, so we'll have: - Mac on the right side - Windows & Linux on the left side For the search behavior, is it possible to have the magnifying glass always stays at the start of the text input, and have the clear icon function once users type text in? Will it be any problems? If it's not clear enough, plz ni me and I'll add more details in the spec. Thanks for your questions!

Hey Mike, Sorry for the typo.... The right direction is: - Mac on the LEFT side - Windows & Linux on the RIGHT side

Thanks Tina. To be clear, is this what you mean?

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review109316 Please recreate your patch from scratch. I had to pull down a set of four commits to get this to apply and one of those four commits was a merge commit which included changes to many other files. In your patch, you should reference chrome://browser/skin/search-indicator-magnifying-glass.svg on all platforms. You may also need to change the -moz-image-region to rect(0, 24px, 24px, 0); to get the magnifying glass to prevent clipping. ::: toolkit/content/widgets/textbox.xml:317 (Diff revision 6) > - > +<!-- Search text box code start --> > <binding id="search-textbox" extends="chrome://global/content/bindings/textbox.xml#textbox"> > <content> > <children/> > - <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context,spellcheck" align="center"> > + <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context,spellcheck" align="center"> > + <xul:image class="textbox-mag-icons" anonid="searchbutton-icon" What is the "mag" in "textbox-mag-icons" an abbreviation for? ::: toolkit/content/widgets/textbox.xml:318 (Diff revision 6) > <binding id="search-textbox" extends="chrome://global/content/bindings/textbox.xml#textbox"> > <content> > <children/> > - <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context,spellcheck" align="center"> > + <xul:hbox class="textbox-input-box" flex="1" xbl:inherits="context,spellcheck" align="center"> > + <xul:image class="textbox-mag-icons" anonid="searchbutton-icon" > + xbl:inherits="src=image,label=searchbuttonlabel,searchbutton,disabled"/> Are you sure that we need to inherit each one of these? We should use as minimal of a list here as possible. ::: toolkit/content/widgets/textbox.xml:324 (Diff revision 6) > + > <html:input class="textbox-input" anonid="input" mozactionhint="search" > xbl:inherits="value,type,maxlength,disabled,size,readonly,placeholder,tabindex,accesskey,mozactionhint,spellcheck"/> > + > <xul:deck class="textbox-search-icons" anonid="search-icons"> > - <xul:image class="textbox-search-icon" anonid="searchbutton-icon" > + <xul:image /> Shouldn't this image still have a class and anonid associated with it? ::: toolkit/content/widgets/textbox.xml:330 (Diff revision 6) > <xul:image class="textbox-search-clear" > onclick="document.getBindingParent(this)._clearSearch();" > label="&searchTextBox.clear.label;" > xbl:inherits="disabled"/> > </xul:deck> > + Please take care not to add any new trailing whitespace. ::: toolkit/content/widgets/textbox.xml:449 (Diff revision 6) > event.stopPropagation(); > ]]> > </handler> > </handlers> > </binding> > - > + <!-- End of Search Text box code --> This comment is unnecessary and can be removed.

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review109996 ::: toolkit/content/widgets/textbox.xml:322 (Diff revision 7) > + <xul:image class="textbox-magnifying-icon" anonid="searchbutton-icon" > + xbl:inherits="src=image,label=searchbuttonlabel,searchbutton,disabled"/> > <html:input class="textbox-input" anonid="input" mozactionhint="search" > xbl:inherits="value,type,maxlength,disabled,size,readonly,placeholder,tabindex,accesskey,mozactionhint,spellcheck"/> > <xul:deck class="textbox-search-icons" anonid="search-icons"> > - <xul:image class="textbox-search-icon" anonid="searchbutton-icon" > + <xul:image /> What is this image used for? It's not clear, especially since it's missing a class or anonid. ::: toolkit/themes/linux/global/textbox.css:74 (Diff revision 7) > - > +/* > .textbox-search-icon { > list-style-image: url(moz-icon://stock/gtk-find?size=menu); > } > - > +*/ Either the code should be removed or kept, but we shouldn't have commented out code. ::: toolkit/themes/linux/global/textbox.css:80 (Diff revision 7) > .textbox-search-icon { > list-style-image: url(moz-icon://stock/gtk-find?size=menu); > } > - > +*/ > +.textbox-magnifying-icon { > + list-style-image: url(chrome://browser/skin/search-indicator-magnifying-glass.svg); "toolkit" is a UI framework that is used by multiple applications (Firefox, SeaMonkey, Thunderbird, etc). Files under "browser" are only used by Firefox. You will need to move this SVG file to toolkit because as-is toolkit can't have any dependencies on /browser (or else SeaMonkey and Thunderbird would fail to build). You can run `hg move browser/themes/shared/search/search-indicator-magnifying-glass.svg toolkit/themes/icons/magnifying-glass.svg` to move the file to toolkit, then you will need to update /toolkit/themes/shared/jar.inc.mn to reference the new file, and remove the old reference in /browser/themes/shared/jar.inc.mn. Then you will also have to search the codebase for any references to chrome://browser/skin/search-indicator-magnifying-glass.svg and replace them with the new reference to the toolkit location of chrome://global/skin/icons/magnifying-glass.svg ::: toolkit/themes/linux/global/textbox.css:86 (Diff revision 7) > + background-image: -moz-image-rect(url("chrome://global/skin/icons/close.svg"), 0, 16, 16, 0); > + background-position: center center; > + background-repeat: no-repeat; > + background-size: 14px; > + height: 14px; > + width: 15px; Why does the -moz-image-rect use 16x16, but the background-size is 14x14, and the width/height is 15x14? ::: toolkit/themes/osx/global/textbox.css:107 (Diff revision 7) > + background-image: -moz-image-rect(url(chrome://browser/skin/search-indicator-magnifying-glass.svg), 0, 24, 24, 0); > + background-size: 14px; > + background-position: center center; > + background-repeat: no-repeat; > + width: 16px; > + height: 14px; Similar question here about different dimensions being used. ::: toolkit/themes/windows/global/textbox.css:97 (Diff revision 7) > + background-image: -moz-image-rect(url(chrome://browser/skin/search-indicator-magnifying-glass.svg), 0, 24, 24, 0); > + background-size: 14px; > + background-position: center center; > + background-repeat: no-repeat; > + width: 16px; > + height: 14px; Ditto. ::: toolkit/themes/windows/global/textbox.css:103 (Diff revision 7) > + background-size: 14px; > + background-position: center center; > + background-repeat: no-repeat; > + width: 16px; > + height: 14px; > + cursor: pointer; Previously we only put the cursor:pointer; property on when the icon wasn't disabled. Why the change?

You can use chrome://global/skin/icons/input-clear.svg as the icon to clear the search field.

Comment on attachment 8828930 [details] Bug 1330115 - Always display the search glass in front of the text with in preferences https://reviewboard.mozilla.org/r/106150/#review110294 We're going to put this work on hold here and come back to it after the reorg work has finished.

(In reply to Mike Conley (:mconley) from comment #12) > Created attachment 8830768 [details] > Magnifying glass / X placement and behaviour > > Thanks Tina. To be clear, is this what you mean? Hi Mike, I've added it to the search spec. Hope it will be more clear! https://mozilla.invisionapp.com/share/ZDAGPK3AF#/218928188_5-0_Search_Field

Hey Manotej - I'm going to unassign you from this one. A full-timer will drive this one through to a resolution. Thanks for your work!

remove whiteboard tag due to this is duplicate