Hang when trying to edit a page on WikiMo with NVDA running

VERIFIED FIXED in Firefox 6

Status

()

Core
Disability Access APIs
--
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: MarcoZ, Assigned: tbsaunde)

Tracking

({hang, regression, verified-aurora})

Trunk
mozilla7
x86
Windows 7
hang, regression, verified-aurora
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox6+ fixed)

Details

Attachments

(1 attachment, 11 obsolete attachments)

(Reporter)

Description

6 years ago
This is a very recent regression:
1. Start NVDA and a current nightly.
2. Go to any page on https://wiki.mozilla.org
3. Click on "Edit this page".
4. The new page loads and should put NVDA into focus mode automatically, focused on the textarea that contains the page text.

Expected: NVDA should allow you to edit the page.
Actual: A hang. Firefox does not crash, but NVDA and Firefox become unresponsive. The only solution is to run Task manager and kill the Firefox.exe process. That will restore NVDA to normal operation.
(Reporter)

Comment 1

6 years ago
Regression window:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=21c304c5f351&tochange=52f72d71dc59

There were 4 bugs in ourmodule that got checked in on that day:
bug 657296
bug 654999
bug 653584
and
bug 658737
(Reporter)

Comment 2

6 years ago
BTW a link that reproduces this for me is
https://wiki.mozilla.org/index.php?title=Accessibility/Meetings/2011-05-25&action=edit

That page currently doesn't exist, and it throws me into that hang immediately.

Comment 3

6 years ago
Firefox doesn't hung, UI works, I can operate on accessible tree, but NVDA keeps silent.

Comment 4

6 years ago
(In reply to comment #1)
> Regression window:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=21c304c5f351&tochange=52f72d71dc59
> 
> There were 4 bugs in ourmodule that got checked in on that day:
> bug 657296
> bug 654999
> bug 653584
> and
> bug 658737

if one of these is guilty then it's bug 653584. Marco, can you try to backout it locally and try?

Comment 5

6 years ago
btw, bug 653584 can hang, for example, TakeFocus(), since mAction can be this and infinite recursion happens. Iirc mAction accessible was never 'this' prior this patch. Trevor, can you fix that?
(Assignee)

Comment 6

6 years ago
Created attachment 535064 [details] [diff] [review]
test patch

make nsLinkableAccessible::BindToParent() never set mActionAcc = this;
(Reporter)

Comment 7

6 years ago
Comment on attachment 535064 [details] [diff] [review]
test patch

This patch definitely fixes the hang! Verified locally by building with and without it.
Attachment #535064 - Flags: feedback+
(Assignee)

Comment 8

6 years ago
Comment on attachment 535064 [details] [diff] [review]
test patch

BEFORE BUG 653584 WE STORED THE ACTION CONTENT AND GOT THE ACCESSIBLE FOR IT WHEN WE NEEDED IT, AND IF THE lINKABLEaCCESSIBLE HAD AN ONCLICK LISTENER ITSELF WE'D SET MaCTIONcONTENT = McONTENT, BUT gETaCTIONaCCESSIBLE() WOULD RETURN NULL IF MaCTIONcONTENT == McONTENT.  sO SINCE WE NOLONGER USE gETaCTIONaCCESSIBLE() WE SHOULD MAKE MaCTIONaCC BE NULL IF THE ACTIONABLE ACCESSIBLE IS THIS SO THAT THE TESTS IF WE SHOULD REDIRECT THE METHOD CALL TO A DIFFERENT ACCESSIBLE STILL DO THE CORRECT THING, NOT INFINETELY RECURSE
Attachment #535064 - Flags: review?(bolterbugz)
Comment on attachment 535064 [details] [diff] [review]
test patch

r=me. Thanks for the explanation.
Attachment #535064 - Flags: review?(bolterbugz) → review+

Comment 10

6 years ago
Comment on attachment 535064 [details] [diff] [review]
test patch


>   // Cache action content.
>   mActionAcc = nsnull;
>   mIsLink = PR_FALSE;
>   mIsOnclick = PR_FALSE;
> 
>   if (nsCoreUtils::HasClickListener(mContent)) {
>-    mActionAcc = this;
>+    mActionAcc = nsnull;

this is crazy, do not land while it's not fixed please

Comment 11

6 years ago
Also, please add mochitests for this, we shouldn't hit this problem twice.
(Assignee)

Comment 12

6 years ago
> this is crazy, do not land while it's not fixed please

ok, your in time, but I'm confused about what you meant by 
 
(In reply to comment #5)
> btw, bug 653584 can hang, for example, TakeFocus(), since mAction can be
> this and infinite recursion happens. Iirc mAction accessible was never
> 'this' prior this patch. Trevor, can you fix that?

I would agree GetActionAccessible() would never return  this, so what do you want mActionAcc to be if not nsnull?
(In reply to comment #10)
> Comment on attachment 535064 [details] [diff] [review] [review]
> test patch
> 
> 
> >   // Cache action content.
> >   mActionAcc = nsnull;
> >   mIsLink = PR_FALSE;
> >   mIsOnclick = PR_FALSE;
> > 
> >   if (nsCoreUtils::HasClickListener(mContent)) {
> >-    mActionAcc = this;
> >+    mActionAcc = nsnull;
> 
> this is crazy, do not land while it's not fixed please

Do you mean Trevor might as well remove the assignment since mActionAcc is already nsnull? Or is it crazy in some other way?

Comment 14

6 years ago
Comment on attachment 535064 [details] [diff] [review]
test patch

this breaks the action number, be careful when you do not 1 to 1 rollback
Attachment #535064 - Flags: review-

Comment 15

6 years ago
(In reply to comment #13)
> (In reply to comment #10)
> > Comment on attachment 535064 [details] [diff] [review] [review] [review]
> > test patch
> > 
> > 
> > >   // Cache action content.
> > >   mActionAcc = nsnull;
> > >   mIsLink = PR_FALSE;
> > >   mIsOnclick = PR_FALSE;
> > > 
> > >   if (nsCoreUtils::HasClickListener(mContent)) {
> > >-    mActionAcc = this;
> > >+    mActionAcc = nsnull;
> > 
> > this is crazy, do not land while it's not fixed please
> 
> Do you mean Trevor might as well remove the assignment since mActionAcc is
> already nsnull? Or is it crazy in some other way?

right, that was a point
(In reply to comment #14)
> Comment on attachment 535064 [details] [diff] [review] [review]
> test patch
> 
> this breaks the action number, be careful when you do not 1 to 1 rollback

Oof! Definitely a good reason to ensure test coverage here.
Comment on attachment 535064 [details] [diff] [review]
test patch

r-=me based on new data (nit, existing test failure, and Alex's catch).
Attachment #535064 - Flags: review+ → review-
(Assignee)

Comment 18

6 years ago
Created attachment 535271 [details] [diff] [review]
(wip) better fix tests are broken

for ome reason getElementById() doesn't seem to be working
(Reporter)

Comment 19

6 years ago
Comment on attachment 535271 [details] [diff] [review]
(wip) better fix tests are broken

>+var acc = document.getElementById("someclickable");

This doesn't give you an accessible, just a dom eelement. Just do 

var acc = getAccessible("someclickable");

This will also do all handling for you.

You can also add this to the array above these new lines, and do regular action testing. Therefore, you would only get this acc for the takeFocus(); test.

>+      is(acc, "image with onclick handler should have 1 action but found " + acc.numActions);

You're missing the expected value here, is() takes 3 parameters. AND it must be acc.numActions. But again, you can ommit this by simply adding this element to the array above this new stuff and let the already existing action testing logic do its magic.

Updated

6 years ago
Blocks: 659826
(Assignee)

Comment 20

6 years ago
Created attachment 535282 [details] [diff] [review]
(next patch

use the actions table for everything but takeFocus() the tests mostly pass, but one dies with a syntax error even though I can't find anything that looks close to a syntax error in the file

Updated

6 years ago
Duplicate of this bug: 659826
(Assignee)

Comment 22

6 years ago
Created attachment 535295 [details] [diff] [review]
patch

correct tests html
Attachment #535064 - Attachment is obsolete: true
Attachment #535271 - Attachment is obsolete: true
Attachment #535282 - Attachment is obsolete: true
Attachment #535295 - Flags: review?(surkov.alexander)

Comment 23

6 years ago
(In reply to comment #22)
> Created attachment 535295 [details] [diff] [review] [review]
> patch
> 
> correct tests html

GetNumActions() should be broken for links still. Please make sure you have mochitest for this case too.

Comment 24

6 years ago
Comment on attachment 535295 [details] [diff] [review]
patch

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

r=me, thanks!

::: accessible/tests/mochitest/actions/test_general.html
@@ +47,3 @@
>        testActions(actionsArray);
> +
> +      //getAccessible("onclick_img").takeFocus();

artifact?

@@ +67,5 @@
>      Mozilla Bug 423409
>    </a>
> +  <a target="_blank"
> +     href="https://bugzilla.mozilla.org/show_bug.cgi?id=659620"
> +     title="hand when trying to edit a page on wikimo withnvda running">

hand -> Hang, withnvda -> with NVDA

@@ +81,5 @@
>      <li id="li_clickable2" onmousedown="">Clickable list item</li>
>      <li id="li_clickable3" onmouseup="">Clickable list item</li>
>    </ul>
> +
> +  <img id="onclick_img" onclick="" src="../moz.png">

maybe comment like <!-- linkable accessibles -->

it makes sense to add tests for
<a href="www">linkable textleaf accessible</a>
<div onclick="">linkable textleaf accessible</div>
Attachment #535295 - Flags: review?(surkov.alexander) → review+

Updated

6 years ago
Assignee: nobody → trev.saunders
Status: NEW → ASSIGNED
(Assignee)

Comment 25

6 years ago
Created attachment 535437 [details] [diff] [review]
patch

address comments on tests
Attachment #535295 - Attachment is obsolete: true
(Assignee)

Comment 26

6 years ago
Created attachment 535493 [details] [diff] [review]
patch to land

correct patch actually includes changes addressing comments this time.
Attachment #535437 - Attachment is obsolete: true

Comment 27

6 years ago
Comment on attachment 535493 [details] [diff] [review]
patch to land


>+
>+      getAccessible("onclick_img").takeFocus();

why do you need that?

>+is(getAccessible("link1").numActions, 1, "links should have one action");
>+is(getAccessible("link2").numActions, 1, "link with onclick handler should have 1 action");

wrong identation. Why you don't use action testing functions like above and prefer to test numActions only?

also I asked to add tests for linkable accessible (text leaf inside accessible with actions), you didn't.
(Assignee)

Comment 28

6 years ago
(In reply to comment #27)
> Comment on attachment 535493 [details] [diff] [review] [review]
> patch to land
> 
> 
> >+
> >+      getAccessible("onclick_img").takeFocus();
> 
> why do you need that?

I thought we want to exercise that there wasn't infinite recursion in takeFocus()? I'm not sure we need it, but I don't see how it hurts.  I gues I could remove it if you like, I thought you were asking if the commenting out  should go away not the whole line.

> >+is(getAccessible("link1").numActions, 1, "links should have one action");
> >+is(getAccessible("link2").numActions, 1, "link with onclick handler should have 1 action");
> 
> wrong identation. Why you don't use action testing functions like above and

ok

> prefer to test numActions only?

well, I don't see how tests for name and  the actionactually hapening can hurt.  

> also I asked to add tests for linkable accessible (text leaf inside
> accessible with actions), you didn't.

ok,  I saw 

> it makes sense to add tests for
> <a href="www">linkable textleaf accessible</a>
> <div onclick="">linkable textleaf accessible</div>

did youask somewhere else then?  do you mean something like <a href="www"><p>test</p></a>?

Comment 29

6 years ago
(In reply to comment #28)

> > >+      getAccessible("onclick_img").takeFocus();
> > 
> > why do you need that?
> 
> I thought we want to exercise that there wasn't infinite recursion in
> takeFocus()? I'm not sure we need it, but I don't see how it hurts.  I gues
> I could remove it if you like, I thought you were asking if the commenting
> out  should go away not the whole line.

then put this test into test_takeFocus.html, because it doesn't look as a test.

> > >+is(getAccessible("link1").numActions, 1, "links should have one action");
> > >+is(getAccessible("link2").numActions, 1, "link with onclick handler should have 1 action");

> > prefer to test numActions only?
> 
> well, I don't see how tests for name and  the actionactually hapening can
> hurt.  

I prefer complex tests in this case. Event if these elements have something common then it makes sense to double test it since these elements have a difference. We don't know whether the ratio of common to difference is permanent.

> > also I asked to add tests for linkable accessible (text leaf inside
> > accessible with actions), you didn't.
> 
> ok,  I saw 
> 
> > it makes sense to add tests for
> > <a href="www">linkable textleaf accessible</a>
> > <div onclick="">linkable textleaf accessible</div>
> 
> did youask somewhere else then?  do you mean something like <a
> href="www"><p>test</p></a>?

the hierarchy is:
  link
    text leaf (linkable accessible)

it's not necessary to put paragraph inside.
(Reporter)

Comment 30

6 years ago
Can we move on on this one? This is needed for Aurora since the hang is in there, too.

Comment 31

6 years ago
Please request approval to land on Aurora for 6 with a risk analysis. Thanks.
tracking-firefox6: ? → +
(Assignee)

Comment 32

6 years ago
Created attachment 536682 [details] [diff] [review]
patch

test linkable accessibles more.  These seem to have passed atleast once for me, but seem to hang other times.
(Reporter)

Comment 33

6 years ago
Comment on attachment 536682 [details] [diff] [review]
patch

>+      getAccessible("onclick_img").takeFocus();
>+

Hm, this will not work because the image itself is not focusable, not even through an onclick handler. It has to have tabindex="0" set.

And then, you should simply do:
       gQueue.push(new takeFocusInvoker("onclick_img"));

before the call to gQueue.invoke();
(Assignee)

Comment 34

6 years ago
Created attachment 537703 [details] [diff] [review]
test cleanup

Alex hopefully this is closer to what you were looking for the first time around.
Attachment #537703 - Flags: review?(surkov.alexander)
(Assignee)

Comment 35

6 years ago
http://hg.mozilla.org/mozilla-central/rev/3b17ef9c3cd8
the acutal fix, and some initial tests landed in  but it seems I never acutally posted the link :(

Comment 36

6 years ago
Comment on attachment 537703 [details] [diff] [review]
test cleanup

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

::: accessible/tests/mochitest/actions/test_general.html
@@ +43,3 @@
>            actionName: "click",
> +          events: CLICK_EVENTS,
> +          numActions: 1

you didn't add a support for numActions property, right?

@@ +67,5 @@
> +is(textChild1.getActionName(0), "jump", "link should have jump action");
> +
> +      var textChild2 = getAccessible("link2").getChildAt(0);
> +      is(textChild2.numActions, 1, "text child accessible of accessible with onclick handler should have 1 action");
> +      is(textChild2.getActionName(0), "click", "text child accessible of an object with an onclick handler should have an onclick handler");

instead all of these, I would allow the usage of objects as "ID" property.

@@ +104,5 @@
>      <li id="li_clickable3" onmouseup="">Clickable list item</li>
>    </ul>
>  
>    <!-- linkable accessibles -->
> +  <img id="onclickImg" onclick="" src="..moz.png">

what's about src change?

btw, I don't get the point of id change.

::: accessible/tests/mochitest/test_takeFocus.html
@@ +52,5 @@
>  
>        gQueue.push(new takeFocusInvoker("aria-link"));
>        gQueue.push(new takeFocusInvoker("aria-link2"));
>        gQueue.push(new takeFocusInvoker("link"));
> +gQueue.push(new takeFocusInvoker("onclick_img"));

fix identation please

@@ +75,2 @@
>    <pre id="test">
>    </pre>

add bug link please
Attachment #537703 - Flags: review?(surkov.alexander) → review-
(Assignee)

Comment 37

6 years ago
Created attachment 538167 [details] [diff] [review]
updated patch address comments
Attachment #537703 - Attachment is obsolete: true
Attachment #538167 - Flags: review?(surkov.alexander)

Comment 38

6 years ago
Comment on attachment 538167 [details] [diff] [review]
updated patch address comments

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

r=me with nits fixed

::: accessible/tests/mochitest/actions.js
@@ +55,5 @@
>      var actionObj = aArray[idx];
>      var accOrElmOrID = actionObj.ID;
>      var actionIndex = actionObj.actionIndex;
>      var actionName = actionObj.actionName;
> +    var actions = "numActions" in actionObj ? actionObj.numActions : -1;

name them actionCount, i.e.

var actionCount = "actionCount" in actionObj ? actionObj.numActions : -1;

since I think eventually nsIAccessible::numAction will be renamed to actionCount

@@ +93,5 @@
>  
>  var gActionsQueue = null;
>  
> +function actionInvoker(aAccOrElmOrId, aActionIndex, aActionName, aActions,
> +                       aEventSeq)

please pass actionCount as a second argument

@@ +109,5 @@
> +      var isThereActions = acc.numActions > 0;
> +      ok(isThereActions,
> +         "No actions on the accessible for " + prettyName(aAccOrElmOrId));
> +      if (!isThereActions)
> +        return INVOKER_ACTION_FAILED;

add an empty line between ok() and if statement

@@ +143,5 @@
>    this.phase = false;
>  
>    this.getID = function getID()
>    {
> +    return aType + " event handling on " + aTarget.id;

ok, fine with me, though probably it should be handled in event.js, i.e. getID() from checker and invoker should be combined.
Attachment #538167 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 39

6 years ago
http://hg.mozilla.org/mozilla-central/rev/3b17ef9c3cd8
the acutal fix, and some initial tests landed in  but it seems I never acutally posted the link :((In reply to comment #38)
> Comment on attachment 538167 [details] [diff] [review] [review]

> 
> var actionCount = "actionCount" in actionObj ? actionObj.numActions : -1;
> 
> since I think eventually nsIAccessible::numAction will be renamed to
> actionCount

mind if I do that and dexpcom it as a low priority thing?

> 
> @@ +93,5 @@
> >  
> >  var gActionsQueue = null;
> >  
> > +function actionInvoker(aAccOrElmOrId, aActionIndex, aActionName, aActions,
> > +                       aEventSeq)
> 
> please pass actionCount as a second argument

you mean the second argument right?

> >    this.getID = function getID()
> >    {
> > +    return aType + " event handling on " + aTarget.id;
> 
> ok, fine with me, though probably it should be handled in event.js, i.e.
> getID() from checker and invoker should be combined.

That seems like a reasonable clean up to me, but my interest in cleaning up js right now is pretty limited, and I'd rather not block this bug on any more refactoring.

Comment 40

6 years ago
(In reply to comment #39)

> > since I think eventually nsIAccessible::numAction will be renamed to
> > actionCount
> 
> mind if I do that and dexpcom it as a low priority thing?

sure I don't mind, though honestly I don't understand why you ask this if you meant interface changes.

> > please pass actionCount as a second argument

> you mean the second argument right?

right.

> > ok, fine with me, though probably it should be handled in event.js, i.e.
> > getID() from checker and invoker should be combined.
> 
> That seems like a reasonable clean up to me, but my interest in cleaning up
> js right now is pretty limited, and I'd rather not block this bug on any
> more refactoring.

btw, I didn't ask to do refactoring right now :)
(Assignee)

Comment 41

6 years ago
Created attachment 538224 [details] [diff] [review]
patch

should address nits.
(Assignee)

Comment 42

6 years ago
(In reply to comment #41)
> Created attachment 538224 [details] [diff] [review] [review]
> patch
> 
> should address nits.

This still causes the tried to run a compile and go script on a cleared scope test failures.     From https://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/10ff69b04b88e06f/87f89aaec17c1aed?lnk=raot it looks like what's happening is that there are event handlers that are still installed when the test is "finished".  I noticed that aAtkObject apparently supported a "targetID" as well as the id attribute so I tried setting that to be getAccessible("link1") in the theory that  maybe events were getting delivered to one but not the other, but that didn't seem to help.
(Assignee)

Comment 43

6 years ago
Created attachment 538820 [details] [diff] [review]
fix only patch

this is only the fix without the first set of test changes.  Since per my last comment I'm a little worried about what might be going on and not sure how long it will take to sort out.  So, I think it may be a good idea to get this on aurora.  If you guys think its worth getting fixed tests on aurora feel free to cancel the approval request.
Attachment #538820 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 44

6 years ago
Comment on attachment 538224 [details] [diff] [review]
patch

Hm, from the patch, I don#t see what could be wrong with those tests except that there are differences in the actionCount name in the array versus the function prototype. Are you sure you have all the parameters from the array to the function call etc. right?
(Reporter)

Comment 45

6 years ago
Comment on attachment 538224 [details] [diff] [review]
patch

>   <a id="link" href="">link</span>

Please put a proper closing </a> instead of </span>

 
>+  <img id="onclick_img" onclick="" src="../moz.png" tabindex="0">

Please end with a proper /> instead of just a > so the tab is properly closed.

And above in the actionsArray, you have:

>+        {
>+          ID: getAccessible("link1").firstChild,
>+          actionName: "jump",
>+          events: CLICK_EVENTS,
>+          actionsCount: 1
>+        },
>+        {
>+          ID: getAccessible("link2").firstChild,
>+          actionName: "click",
>+          events: CLICK_EVENTS,
>+          actionsCount: 1
>         }

Is getting the ID like this really supposed to work?
(Reporter)

Comment 46

6 years ago
Created attachment 539441 [details] [diff] [review]
Further test adjustment and fixes, but still not working
(Assignee)

Comment 47

6 years ago
(In reply to comment #46)
> Created attachment 539441 [details] [diff] [review] [review]
> Further test adjustment and fixes, but still not working
Marco  said on irc that the problem was when we try to focus link2.  when I broke on DoAction(0 in gdb I found that nsAccessible::DoAction() was being called not anything in nsLinkableAccessible.  nothing appears immediately wrong with nsAccessible::DoAction() however I notice that  nsAccessible::DoCommand() just calls NS_DISPATCH_RUNNABLE_METHOD_ARG2() with a function that sends mouse up / down events, but I'm not seeing why a click event will get generated.
(Reporter)

Comment 48

6 years ago
(In reply to comment #43)
> Created attachment 538820 [details] [diff] [review] [review]
> fix only patch
> 
> this is only the fix without the first set of test changes.  Since per my
> last comment I'm a little worried about what might be going on and not sure
> how long it will take to sort out.  So, I think it may be a good idea to get
> this on aurora.  If you guys think its worth getting fixed tests on aurora
> feel free to cancel the approval request.

I second this request to take the code-only fix on Aurora and fix the hang that will affect a lot of users using WordPress, Wikipedia editing and others. We've discussed this internally and also Alex Surkov as the module owner agrees it's worth taking the code-only fix on Aurora now.
(Reporter)

Updated

6 years ago
Attachment #538820 - Attachment is obsolete: true
Attachment #538820 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 49

6 years ago
Comment on attachment 535493 [details] [diff] [review]
patch to land

This is the proper patch with test coverage that has been on Mozilla-Central for a while. It fixes the hang, is low risk, and it has test coverage. I will move the additional test coverage to a separate bug where we can also work on what those additional tests apparently uncover as a separate bug, but this one here fixes this particular bug and should go to Aurora ASAP so the hang gets off users' annoyance.
Attachment #535493 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 50

6 years ago
Resolving fixed because the initial fix landed (see comment #39) and fixes the bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Attachment #536682 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Attachment #538167 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Attachment #538224 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Attachment #539441 - Attachment is obsolete: true
(Reporter)

Updated

6 years ago
Flags: in-testsuite+
(Reporter)

Updated

6 years ago
Depends on: 664443
(Reporter)

Updated

6 years ago
Target Milestone: --- → mozilla7

Comment 51

6 years ago
Comment on attachment 535493 [details] [diff] [review]
patch to land

Approved for mozilla-aurora
Attachment #535493 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Comment 52

6 years ago
Pushed on Trevor's behalf to mozilla-aurora: http://hg.mozilla.org/releases/mozilla-aurora/rev/addb57fc93b5
status-firefox6: --- → fixed
(Reporter)

Comment 53

6 years ago
Verified fixed in Aurora build Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a2) Gecko/20110616 Firefox/6.0a2
Keywords: verified-aurora
Set status to verified based on the above comment.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.