338378 - When customizing toolbars, search box has grey text and I-beam cursor and is hard to drag

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

Looks like the adding of gray text to the search bar while it's in the palette makes it possible to select that text. The search bar binding constructor should probably not fire if the search bar is in the palette. There was a sort of hacky fix for this at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/search/content/search.xml&rev=1.56#461 , but I'm not sure that it still works, and there isn't anything that prevents the normal startup in the search bar constructor itself.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

This also makes moving the search bar much more difficult. When customizing the toolbars, the "hand" icon is only shown over the engine's favicon and the magnifying glass - otherwise it's a cursor to select text. Combined with the new flex behaviour for the bar length, the majority of the search bar is no use for rearranging. As Gavin points out, this is a regression from v1.5.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

This isn't going to block release, but we'd take the polish patch if it's simple.

Comment 3

[Dave Townsend [:mossop]]

I'm a man with a plan.

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

It turns out that the grey text had nothing to do with this and in fact this was a regression from bug 337292 adding extra elements between the textbox and the toolbaritem.

autocomplete.css uses an insanely specific rule to hide the input element of a textbox within a toolbaritem when toolbars are being customized.

This patch simply uses a simpler form of this rule for the specific search bar case.

Also included is code to disable the search bar initialisation when in customization, though that is not actually necessary to resolve this bug so it might be better to leave that out.

Comment 5

[Dave Townsend [:mossop]]

There is a different patch posted in bug 348947 that appears to be for the same issue as this.

Comment 6

[Dave Townsend [:mossop]]

*** Bug 348947 has been marked as a duplicate of this bug. ***

Comment 7

[Peter Kasting]

Changing target milestone to Fx2 and modifying summary since bug 348947 was duped against this.  I think the effects seen on that bug are severe enough to reconsider blocking, so re-requesting.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Connor apparently changed his mind on whether or not this blocks final release. OK, I guess. Not blocking beta2, though.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 233307 [details] [diff] [review]
patch rev 1

>Index: browser/themes/winstripe/browser/browser.css

>+toolbarpaletteitem #searchbar html|*.textbox-input {
>+  visibility: hidden;
>+}

Use |#wrapper-search-container #searchbar html|*.textbox-input| as discussed on IRC, and r=me.

Comment 10

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Changed selector as requested. Carrying over review.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/components/search/content/search.xml 	1.85
mozilla/browser/themes/pinstripe/browser/browser.css 	1.30
mozilla/browser/themes/winstripe/browser/browser.css 	1.44

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 234578 [details] [diff] [review]
patch rev 2

a=mconnor on behalf of drivers for 1.8 branch checkin

Comment 13

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/themes/winstripe/browser/browser.css 	1.17.2.46
mozilla/browser/components/search/content/search.xml 	1.37.2.57
mozilla/browser/themes/pinstripe/browser/browser.css 	1.11.4.36