347576 - Go and Search buttons have no text

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Patrick Thompson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060805 BonEcho/2.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060805 BonEcho/2.0b1

The 'value="Go"' attribute was recently removed from the toolbarbutton-text element of the Go button. I've got two themes that skin the Go button with the text and not an image. Any chance on re-implementing the toolbarbutton-text value and then just doing a display="none" in the default theme? Also, an argument could be made that an XUL image element without a value attribute is akin to an HTML image element without an ALT tag; albeit, not entirely the same issue.

Reproducible: Always

Steps to Reproduce:
Interrogate the Go-button using DOMi.

Actual Results:  
The value attribute had recently been removed.

Expected Results:  
A 'value="Go"' attribute should be present.

Comment 1

[Adam Guthrie]

*** This bug has been marked as a duplicate of 347454 ***

Comment 2

[Chris Ilias [:cilias]]

Adam, why is this a dup of bug 347454? Bug 347454 is a tracking bug. Did you mean to make this block bug 347454?

Comment 3

[Adam Guthrie]

Yes, I did. Apologies.

Comment 4

[Simon Bünzli]

AFAICT the new Go button is supposed to be graphical only (the current design doesn't allow for text to be attached and visually impaired people are supposed to hit Enter instead anyway).

And for theming you've always been supposed to use #go-button instead of [value="Go"] (otherwise your theme simply breaks for non-English locales).

Probably INVALID.

Comment 5

[Rob Marshall [tH]]

Having buttons disappear when the toolbar is in text-only mode doesn't make sense to me.

Comment 6

[Patrick Thompson]

In addition to adding the value back to the Go button it would also makes since to include to value="Search" for the search-Go button at the same time.

Simon, wouldn't a non-English locales simply need to add the Go-button (and search-Go) button values to their internationalization conversions?

Comment 7

[Simon Bünzli]

I seem to have misunderstood your request (I thought you were styling the Go button depending on its value). Of course the value should be returned for those themes wishing to use both text and imagery or even text only.

Comment 8

[Mike Connor [:mconnor]]

The go button isn't going to be a separate button, I'm not sure its going to be useful to have a label for something that isn't focusable or configurable.

Comment 9

[Simon Bünzli]

AFAICT the Go button never was focusable and most users never moved it from beside the address bar. So themes not liking the new attached button style might want to continue as if nothing had happened internally (which otherwise isn't possible without major hacking). Besides, having a label also easily allows for a larger target (for Fitts'y themes not wanting to ship locales as well).

Comment 10

[Patrick Thompson]

Attachment: Go Button theme example with Fx (top) and SeaMonkey (bottom)

Attachment depicts how the theme uses the Go button text in Fx and SM

Comment 11

[Mike Connor [:mconnor]]

Need to add the label back, but not display in the default theme.

Comment 12

[Ryan Flint [:rflint] (ping via IRC for reviews)]

I'll roll this into my patch for bug 347754.

Comment 13

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v1

I wasn't able to test this on Mac, but the change is essentially the same and *should* work. Of course I'd sleep much easier if someone tried it out before landing. ;)

Comment 14

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v2

Unbitrotted patch.

Comment 15

[Pam Greene]

(In reply to comment #13)
> I wasn't able to test this on Mac, but the change is essentially the same and
> *should* work. Of course I'd sleep much easier if someone tried it out before
> landing. ;)

Looks fine on Mac -- which is to say, no visible difference, but DOMi shows the labels.

Comment 16

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v3

Updated patch after some excellent guidance from Mano.

Comment 17

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Unbitrot (again)

Comment 18

[Asaf Romano (gone)]

1.8 branch:
mozilla/browser/base/content/browser.xul 1.268.2.58
mozilla/browser/components/search/content/search.xml 1.37.2.55
mozilla/browser/locales/en-US/chrome/browser/browser.dtd 1.25.2.26
mozilla/browser/locales/en-US/chrome/browser/searchbar.dtd 1.1.2.3
mozilla/browser/themes/pinstripe/browser/browser.css 1.11.4.28
mozilla/browser/themes/winstripe/browser/browser.css 1.17.2.39