501608 - menu popup sounds are played at showing the autoscrolling popup

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Adrian]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

When using autoscrolling (middle mouse button), you hear the windows "context menu" sound (if configured in Control Panel, by default there is none).
Same happens when the "Confirm close" window is shown.
However, neither of those actions are even closely related to context menus so that sound is absolutely inappropriate there.

Reproducible: Always

Steps to Reproduce:
1. Configure a "context menu" sound in windows control panel
2. Use autoscrolling in Firefox
3. Hear the annoying sound
Actual Results:  
Context menu sound

Expected Results:  
No context menu sound

Comment 1

[Adrian]

Forums thread about the bug:
http://forums.mozillazine.org/viewtopic.php?f=38&t=1327265

Comment 2

[Kevin Brosnan]

In Windows 7 the sound is called menu popup. Can reproduce with Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5

Comment 3

[u88484]

This is by design, see bug 83056

Comment 4

[Adrian]

This *CANNOT* be by design. The sounds plays when you use AUTOSCROLLING which is not an even which even *has* a system sound. You can open word or interne explorer and use their autoscrolling without a sound.

Comment 5

[u88484]

(In reply to comment #4)
> This *CANNOT* be by design. The sounds plays when you use AUTOSCROLLING which
> is not an even which even *has* a system sound. You can open word or interne
> explorer and use their autoscrolling without a sound.

Sorry, I focused on the,
"Actual Results:  
Context menu sound

Expected Results:  
No context menu sound"

and took it as there was sound in the context menu and you didn't want sound there.

Masayuki, any clue here?

Comment 6

[Robert Strong (they/them - no direct email)]

This is not a shell integration bug and is likely a widget bug since it involves the scrollbar.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ugh... The autoscrolling marker is a popup element that is same as menupopup element. So, this bug might be INVALID on XUL specification layer.

I have two ideas:

1. We should play the menu popup sound only when it's menupopup element. But the old MDC document said that the popup element is same as menupopup element. So, is the popup element an old style of the menupopup element? If so, we should not take this approach.

2. Fx should use panel element for the autoscrolling marker.

I guess that some add-ons use the popup element for menus, then, we should not change the current behavior. So, 2nd idea is better, I think.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://mxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#349

> 349 /********* popup & menupopup ***********/
> 350 
> 351 /* <popup> is deprecated.  Only <menupopup> and <tooltip> are still valid. */ 
> 352 
> 353 popup,
> 354 menupopup {
> 355   -moz-binding: url("chrome://global/content/bindings/popup.xml#popup");
> 356   -moz-box-orient: vertical;
> 357 }

I found this. So, probably, the 2nd way is correct.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0

simple fix

Comment 11

[Neil Deakin]

Comment on attachment 386335 [details] [diff] [review]
Patch v1.0

(From update of attachment 386219 [details] [diff] [review])
Seems OK. Does it affect key events firing while scrolling?

You might want to ask Chris Thomas, Mano or the other Neil to review instead as
they might have more insight into if this would break something. It is the
right fix to do though.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #11)
> Does it affect key events firing while scrolling?

Excuse me, what did you mean?

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #12)
> (In reply to comment #11)
> > Does it affect key events firing while scrolling?
> 
> Excuse me, what did you mean?

Ah, ok, I see. The patch changes some behaviors at key pressing. I need more works for this.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Is this a regression?

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.0

Enn:

This fixes the unexpected behavior changes at the key events. If ESC key is pressed, autoscrolling is finished, this is same behavior. Otherwise, the all keys are eaten by the autoscrolling, e.g., arrow keys, page up/down keys, home/end keys and alt key on Windows.

Comment 16

[Neil Deakin]

Panels have a default listener on keypress which handles the escape key. This could be used, no?

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #16)
> Panels have a default listener on keypress which handles the escape key. This
> could be used, no?

Er, yes.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v3.0

ok, this uses the key handling of nsXULPopupManager. But isn't the previous patch (v2.0) better? That is simpler than this? Please choose v2.0 or v3.0 at review.

Comment 19

[Neil Deakin]

What was the problem with keys with the very first patch?

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #19)
> What was the problem with keys with the very first patch?

E.g., Alt key moves the focus to menu bar, The arrow keys, Home, End, Page Up and Page down keys scroll the focused document. All keys excepting ESC key were usable normally during autoscrolling.

Comment 21

[Neil Deakin]

Comment on attachment 386868 [details] [diff] [review]
Patch v3.0

It seems like it would be simpler to make this a browser_chrome test: just load a long page stored in a data url in it and send keys at it.

>+<body id="body" onload ="setTimeout(ensureFocus,0); setTimeout(runTests,0);"

You should just use the focus event, not the load event.

>+function runTests()
>+{
>+  gExpectedKeyEvents = kAllKeyEvents;
>+  sendKey("A");
>+  // start autoscrolling
>+  synthesizeMouse(document.getElementById("body"), 10, 10, { button: 1} );
>+  gExpectedKeyEvents = kNoKeyEvents;
>+  sendKey("A");
>+  sendKey("VK_DOWN");
>+  sendKey("VK_RETURN");
>+  sendKey("VK_ENTER");
>+  sendKey("VK_HOME");
>+  sendKey("VK_END");
>+  sendKey("VK_TAB");
>+  sendKey("VK_ENTER");
>+  // finish autoscrolling
>+  gExpectedKeyEvents = kKeyUpEvent; // key down and key press should be eaten
>+  sendKey("VK_ESCAPE");
>+  gExpectedKeyEvents = kAllKeyEvents;
>+  sendKey("A");
>+  window.close();
>+}
>+
>+function finish()
>+{
>+  gPrefSvc.setBoolPref(kPrefName_AutoScroll, kAutoScrollingEnabled);
>+  window.opener.wrappedJSObject.SimpleTest.finish();
>+}
>+
>+function sendKey(aKey)
>+{
>+  gKey = aKey;
>+  gDispatchedKeyEvents = kNoKeyEvents;
>+  synthesizeKey(aKey, {});
>+  is(gDispatchedKeyEvents, gExpectedKeyEvents,
>+     "unexpected key events were dispatched or not dispatched: " + gKey);
>+}
>+
>+function onKey(aEvent)
>+{
>+  var keyFlag;
>+  if (aEvent.type.toLowerCase() == "keydown")
>+    keyFlag = kKeyDownEvent;

Should already be lowercase, no?

>+  else if (aEvent.type.toLowerCase() == "keypress")
>+    keyFlag = kKeyPressEvent;
>+  else if (aEvent.type.toLowerCase() == "keyup")
>+    keyFlag = kKeyUpEvent;
>+  else {
>+    ok(false, "Unknown events: " + aEvent.type);
>+    return;
>+  }
>+  gDispatchedKeyEvents |= keyFlag;
>+  is(gExpectedKeyEvents & keyFlag, keyFlag, aEvent.type + " fired: " + gKey);

The first argument is supposed to be the actual result not based on the expected result.

I don't really understand this function though. It seems like it should just store that the event occured and do any comparisons needed after the synthesizeKey is finished.

Also, can you add a check to ensure that the event target is right?

>+              case "keypress": {
>+                if (aEvent.keyCode == aEvent.DOM_VK_ESCAPE) {
>+                  // the escape key will be processed by
>+                  // nsXULPopupManager::KeyPress and the panel will be closed.
>+                  // So, nothing to do here.
>+                  break;
>+                }
>+                // don't break here.

No need to listen to the keypress event at all.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #21)
> >+              case "keypress": {
> >+                if (aEvent.keyCode == aEvent.DOM_VK_ESCAPE) {
> >+                  // the escape key will be processed by
> >+                  // nsXULPopupManager::KeyPress and the panel will be closed.
> >+                  // So, nothing to do here.
> >+                  break;
> >+                }
> >+                // don't break here.
> 
> No need to listen to the keypress event at all.

No, if browser.xml doesn't eat keypress events, Ctrl+Tab is worked during autoscrolling. And probably, other features (including implemented by addons) can be processed during autoscrolling.

Comment 23

[Neil Deakin]

(In reply to comment #22)
> > No need to listen to the keypress event at all.
> 
> No, if browser.xml doesn't eat keypress events, Ctrl+Tab is worked during
> autoscrolling. And probably, other features (including implemented by addons)
> can be processed during autoscrolling.

Ah, yes. I missed that the break wasn't there.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v4.0

ok, I rewrite the test in the browser-chrome-test. However, it wasn't passed on Linux. I'm not sure the reason. The autoscrolling isn't started by the systhesizeMouse with button:1 (it means middle click).

When I added an input element to the data URI, the input element cannot get at onload event *only* on Linux. Therefore, I disabled the test on Linux.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v4.1

adding a line for cleaning-up the tested tab.

Comment 26

[Neil Deakin]

Comment on attachment 390214 [details] [diff] [review]
Patch v4.1

>+  function onPageShow() {
>+    gBrowser.removeEventListener("pageshow", onPageShow, false);
>+
>+    gBrowser.addEventListener("load", onLoad, false);
>+  }

What pageshow event is being listened for here?

This test doesn't ensure that the window for the loaded window is focused, which may possibly be related to why it fails on linux. If the window isn't focused, then the key events will fail.

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #26)
> This test doesn't ensure that the window for the loaded window is focused,
> which may possibly be related to why it fails on linux. If the window isn't
> focused, then the key events will fail.

Ah, you're right for the not-ensured the window is focused.

But the failure on Linux is another issue. Because the key events come to the tests. The issue is that the autoscrolling isn't started by the middle click emulation.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The test failure log on Linux.

> Running chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js...
> TEST-PASS | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: A
> TEST-PASS | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: A
> TEST-PASS | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: A
> TEST-PASS | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: A
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: A - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: A - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: A - Got 4, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: A - Got 7, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: VK_DOWN - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: VK_DOWN - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: VK_DOWN - Got 4, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_DOWN - Got 7, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: VK_RETURN - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: VK_RETURN - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: VK_RETURN - Got 4, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_RETURN - Got 7, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: VK_ENTER - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: VK_ENTER - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: VK_ENTER - Got 4, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_ENTER - Got 7, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: VK_HOME - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: VK_HOME - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: VK_HOME - Got 4, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_HOME - Got 7, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: VK_END - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: VK_END - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keyup fired: VK_END - Got 4, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_END - Got 7, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: VK_TAB - Got 1, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keypress fired: VK_TAB - Got 2, expected 0
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_TAB - Got 3, expected 0
> TEST-PASS | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_ENTER
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: VK_ESCAPE - Got 0, expected 4
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | unexpected key events were dispatched or not dispatched: A - Got 0, expected 7

Comment 29

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v4.2

Comment 30

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/8cd49a8cbb88

Comment 31

[Dão Gottwald [:dao]]

The test fails intermittently:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1249025902.1249028667.5394.gz

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #31)
> The test fails intermittently:
> 
> http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1249025902.1249028667.5394.gz

Both on Windows and Mac, the intermittent failures has similar log:

Mac:

> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_491168.js | Timed out
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_491168.js | Referrer retrieved via getTabState matches referrer set via loadURI. - Got undefined, expected http://www.example.net/?1249027094634
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_bug471962.js | Timed out
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: A - Got 1, expected 0

Win:

> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/places/tests/perf/browser_ui_bookmarks_sidebar.js | Timed out
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_491168.js | Timed out
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_491168.js | document.referrer matches referrer set via setTabState. - Got , expected http://www.example.net/?0.030832644883006965
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_491168.js | document.referrer is still correct after closing and reopening the tab. - Got , expected http://www.example.net/?0.030832644883006965
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_bug471962.js | Timed out
> TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_keyevents_during_autoscrolling.js | keydown fired: A - Got 1, expected 0

Perhaps, browser_491168.js failure causes the both failures browser_bug471962.js and browser_keyevents_during_autoscrolling.js.

Note that the log of browser_keyevents_during_autoscrolling.js said that the autoscrolling wasn't started by synthesized middle click event.

Comment 33

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Additional patch v1.0

This patch should fix the intermittent failure. This checks whether the autoscrolling is started correctly or not. If failed, this patch starts the autoscrolling forcedly.

I tried to call browser.startScroll at onload event, but it doesn't work fine because event.originalTarget.ownerDocument is null at that time. For fix it, I use setTimeout hack at onload event. Then, the patch works fine. And the hack fixes the tests didn't work fine on Linux.

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm preparing to back this out because of the frequent mochitest-browser-chrome orange (bug 507698).

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Backed out:
http://hg.mozilla.org/mozilla-central/rev/1708ccd0ffe4
http://hg.mozilla.org/mozilla-central/rev/765f0b876846

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since it wasn't the cause of the orange, I relanded it:
http://hg.mozilla.org/mozilla-central/rev/d4f252efece9

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #36)
> Since it wasn't the cause of the orange, I relanded it:
> http://hg.mozilla.org/mozilla-central/rev/d4f252efece9

Thank you, but the additional patch should be landed I think. Because it helps to read the log and it can suppress the unexpected failure by other tests.

Comment 38

[Neil Deakin]

Comment on attachment 391901 [details] [diff] [review]
Additional patch v1.0

>+    var browser = gBrowser.selectedTab.linkedBrowser;
>+    if (!browser.isAutoScrolling) {
>+      // If we failed to start autoscrolling by the synthesized middle click,
>+      // we should start it forcedly.
>+      todo(false, "failed to start autoscrolling by synthesized middle click");
>+      var event = {
>+        originalTarget: root,
>+        screenX: 0,
>+        screenY: 0
>+      };
>+      browser.startScroll(event);
>+      ok(browser.isAutoScrolling, "cannot start autoscrolling");
>+      if (!browser.isAutoScrolling) {
>+        onFinishTests();
>+        return;
>+      }
>+    }

It doesn't seem to me that we should do a workaround in a test if the original attempt failed. We should instead find out why it failed and fix that.

But the problem wasn't caused by this test, right? So maybe this is no longer necessary?

Comment 39

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 391901 [details] [diff] [review]
Additional patch v1.0

ok, we should not land the workaround for the test. and fortunately, looks like the random failure possibility is very low.

Comment 40

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 391115 [details] [diff] [review]
Patch v4.2

This is a regression of Fx3.5. The menu popup sound should not be played at popupping the autoscrolling marker.

The risk is low, this patch changes the element of the marker from "popup" (obsolete element) to "panel" element. And this patch eats the all key events during autoscrolling excepting that the VK_ESCAPE which is used for stopping the autoscrolling.

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Oh, I forgot an important thing. The new test was disabled on the trunk, so, even if the patch will get the approval, I'll disable the test on 1.9.1 branch too.

Comment 42

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #41)
> Oh, I forgot an important thing. The new test was disabled on the trunk, so,
> even if the patch will get the approval, I'll disable the test on 1.9.1 branch
> too.

Oops, please ignore this comment, the disabling some new tests is another bug thing. The new tests for this bug are enabled on non-Linux platforms on the trunk.

Sorry for the spam.

Comment 43

[:Paolo Amadini]

On my Windows XP machine, if I run "browser_keyevents_during_autoscrolling.js"
alone, passing a --test-path to run_tests.py, it fails deterministically.

Only if the test is run after "browser_bug471962.js", it succeeds.

See also bug 486342. Any idea?

Comment 44

[:Paolo Amadini]

(In reply to comment #43)
> On my Windows XP machine, if I run "browser_keyevents_during_autoscrolling.js"
> alone, passing a --test-path to run_tests.py, it fails deterministically.

See bug 512100 for a possible workaround, that actually includes portions of
Masayuki's attachment 391901 [details] [diff] [review].

Comment 45

[Daniel Veditz [:dveditz]]

Comment on attachment 391115 [details] [diff] [review]
Patch v4.2

Doesn't seem like the kind of fix we have to take on the stable branch. 1.9.1 approval denied.

Comment 46

[Bryan]

Could this have broken AutoScroll?  In the last few builds I've had AutoScroll not work at all on more then half of the tabs I have open.  What happens is I switch to a random tab and click my scroll wheel and I get no response, no AutoScroll icon, nothing!  I can't find STR as it appears to be completely random.  Others are starting to report this as well: http://forums.mozillazine.org/viewtopic.php?f=23&t=1497265 (towards the bottom)

BUILD: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090922 Minefield/3.7a1pre (.NET CLR 3.5.30729) ID:20090922151415

~B