On about:home, searchEngineLogo alt-text is the string "undefined"

VERIFIED FIXED in Firefox 18

Status

()

defect
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

({regression})

Trunk
Firefox 19
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox17 unaffected, firefox18+ fixed)

Details

(Whiteboard: [about-home])

Attachments

(1 attachment, 2 obsolete attachments)

When I visit about:home, the Google logo to the left of the search-box sometimes doesn't render right away, and we show the alt text.

The alt text is "undefined".  This text is a bit unsettling to see on your home-page.

You can also see this if you visit about:home, right-click the google logo, pick "Inspect Element", and click the "markup" button to show the underlying HTML version of the DOM tree.  It looks like this:
<img xmlns="http://www.w3.org/1999/xhtml" id="searchEngineLogo" src="data:image/png;base64,[snipping out the giant data URI]" alt="undefined" />
The place where we set logoElt.alt is here:

> 141     let logoElt = document.getElementById("searchEngineLogo");
> 142     logoElt.src = searchEngineInfo.image;
> [...]
> 147     logoElt.alt = searchEngineInfo.name;
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/abouthome/aboutHome.js#141

...so searchEngineInfo.name is probably undefined.
Yeah, it's defined at the top of this file:

>  7 const SEARCH_ENGINES = {
>  8   "Google": {
>  9     image: "[data URI]",
[...]
> 71   , params: "source=hp&channel=np"
> 72   }

No "name" field there, hence searchEngineInfo.name is undefined.

https://mxr.mozilla.org/mozilla-central/source/browser/base/content/abouthome/aboutHome.js#7
I think we just want to use "searchEngineName" (the index that we use into SEARCH_ENGINES -- "Google" as defined on line 8, quoted in comment 2).
(Looks like this was a regression from bug 749477, which added this use of ".name" on an entry from SEARCH_ENGINES:
 https://hg.mozilla.org/mozilla-central/rev/08c90c0d3f55#l1.89
whereas previously we were using ".name" on something else)
Blocks: 749477
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Posted patch fix (obsolete) — Splinter Review
Attachment #674967 - Flags: review?(mak77)
(Confirmed locally that this fixes the alt text, w/ Inspect Element, and also by clearing image cache, reloading, and watching the alt text flash before the image loads.)
Posted patch fix w/ test (obsolete) — Splinter Review
Here's the fix again, with a testcase added to browser_aboutHome.js.

Verified locally that the testcase's final check[1] fails without the fix, and passes after I apply the fix.

[1] isnot(altText, "undefined", ...)
Attachment #674967 - Attachment is obsolete: true
Attachment #674967 - Flags: review?(mak77)
Attachment #674984 - Flags: review?(mak77)
Verified that Firefox 17 is unaffected. (using Inspect Element on about:home)

So, this is a new bug in Firefox 18, which is currently in Aurora channel. Once this lands on trunk, I think we should take this on Aurora as well -- it's a low-risk fix, and we might as well avoid shipping this bug to our Beta & Release users.
Comment on attachment 674984 [details] [diff] [review]
fix w/ test

Review of attachment 674984 [details] [diff] [review]:
-----------------------------------------------------------------

OOps, thank you!

::: browser/base/content/test/browser_aboutHome.js
@@ +78,5 @@
> +
> +    let searchEngineLogoElt = doc.getElementById("searchEngineLogo");
> +    ok(searchEngineLogoElt, "Found search engine logo");
> +
> +    var altText = searchEngineLogoElt.alt;

nit: let
Attachment #674984 - Flags: review?(mak77) → review+
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [about-home]
Fixed that & landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/a46d4906eee8

I'll request Aurora approval once this has cycled on tbpl.
Attachment #674984 - Attachment is obsolete: true
Attachment #675110 - Flags: review+
Flags: in-testsuite+
Attachment #675110 - Attachment description: fix w/ test & review-comment addressed → fix w/ test & review-comment addressed [r=mak]
Comment on attachment 675110 [details] [diff] [review]
fix w/ test & review-comment addressed [r=mak]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 749477

User impact if declined: Incorrect/broken alt text ("undefined") on search-engine logo <img> element, on about:home 

Testing completed (on m-c, etc.): Verified the fix locally, landed on m-c (and patch includes automated test to verify that it's fixed there)

Risk to taking this patch (and alternatives if risky):  Negligible.

String or UUID changes made by this patch: None.
Attachment #675110 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a46d4906eee8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Attachment #675110 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed for FF 18.b4 Windows 7 x63 and Ubuntu x86, using Inspect Element on about:home.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0(20121212073002)
Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/20100101 Firefox/18.0(20121212073002)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.