Last Comment Bug 805206 - Social API keyboard-accessible menuitem is blank
: Social API keyboard-accessible menuitem is blank
Status: RESOLVED FIXED
[Fx17][qa-]
:
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 19
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
: Shane Caraveo (:mixedpuppy)
Mentors:
Depends on:
Blocks: 801040
  Show dependency treegraph
 
Reported: 2012-10-24 14:38 PDT by Jared Wein [:jaws] (please needinfo? me)
Modified: 2012-12-06 12:26 PST (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed


Attachments
Patch (3.04 KB, patch)
2012-10-24 14:38 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch v1.1 (2.90 KB, patch)
2012-10-24 14:50 PDT, Jared Wein [:jaws] (please needinfo? me)
felipc: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Jared Wein [:jaws] (please needinfo? me) 2012-10-24 14:38:59 PDT
Created attachment 674841 [details] [diff] [review]
Patch

Bug 801040 tweaked the keyboard-accessible menu code, and removed the command. The command attribute did server a purpose though, as it set the label on the menuitem.
Comment 1 Jared Wein [:jaws] (please needinfo? me) 2012-10-24 14:50:41 PDT
Created attachment 674843 [details] [diff] [review]
Patch v1.1
Comment 2 :Felipe Gomes (needinfo me!) 2012-10-24 20:48:46 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbfcd1d5296
Comment 3 Jared Wein [:jaws] (please needinfo? me) 2012-10-24 20:59:30 PDT
Comment on attachment 674843 [details] [diff] [review]
Patch v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): needed to fix a regression by bug 801040 (for social api)
User impact if declined: blank menuitem in tools menu
Testing completed (on m-c, etc.): locally and landed on m-c
Risk to taking this patch (and alternatives if risky): none expected
String or UUID changes made by this patch: none
Comment 4 Phil Ringnalda (:philor) 2012-10-24 22:36:40 PDT
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbfcd1d5296 - Impact when landed: 10 browser-chrome failures.
Comment 5 Phil Ringnalda (:philor) 2012-10-24 22:54:42 PDT
Oh, and my very favoritest sort of bustage: for historical reasons, Talos treats any run which has the string ERROR in allcaps in the log as a failure, and in Mac Talos other and dirtypaint you were triggering a whole lot of "Exception... "'ReferenceError: separator is not defined' when calling method: [nsIRunnable::run]"  nsresult: "0x8057001c (NS_ERROR_XPC_JS_THREW_JS_OBJECT)"
Comment 6 Jared Wein [:jaws] (please needinfo? me) 2012-10-25 12:37:17 PDT
Relanded with the fixed JS,
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a3dbca1ca7d
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-10-25 18:27:04 PDT
https://hg.mozilla.org/mozilla-central/rev/6a3dbca1ca7d
Comment 8 Lukas Blakk [:lsblakk] use ?needinfo 2012-10-26 11:16:39 PDT
Comment on attachment 674843 [details] [diff] [review]
Patch v1.1

Approving for uplift (even though untracked) since it's needed for the Social API launch in 17.
Comment 10 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-12-04 14:46:45 PST
Does this fix need any QA?
Comment 11 Jared Wein [:jaws] (please needinfo? me) 2012-12-06 12:26:43 PST
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #10)
> Does this fix need any QA?

We should be all set here.

Note You need to log in before you can comment on or make changes to this bug.