Launchpad bug pages trigger caret browsing in Firefox and other Gecko browsers

LP: https://bugs.launchpad.net/bugs/107247

Steps to reproduce:
1. In Firefox or Epiphany, open any launchpad bug page, such as the one above.
2. Wait for the Subscribers box to finish loading.
3. Press the Up or Down arrow keys.

Result: firefox switches to caret browsing mode.

We see this on ffox 3.0/3.5 and mozilla-central builds.

Looks like the linked site changed something since this bug was filed so that it no longer qualifies as a test case. Does anyone have another test case?

hmmm. i think you need to be member of the launchpad beta testers to see the version affected. i will try to get something ...

got more info: seems it should also happen if you are not in the launchpad beta testers. however, i was told that you need to be logged in to reproduce.

maybe also helpful that this commit makes it goes away:

http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/revision/9000

Sorry, but there's no obvious link between that commit and caret browsing.

maybe also check https://bugs.edge.launchpad.net/bugs/404088 (and https://bugs.edge.launchpad.net/bugs/392138) which was the bug that caused the above fix for this as a side-effect to land. They are talking about focusing some hidden element. does that ring a bell?

No, but I'll CC enndeakin as being the person who knows most about focusing...

> Result: firefox switches to caret browsing mode.

That seems unlikely. Do you actually mean that caret browsing is enabled (and that the preference is set), or just that the caret appears? Is the text editable?

Please describe what actually happens.

A testcase is needed here as well.

we are working on a testcase ...

But it really triggers caret browsing. I just tried and this are the instructions

 1. caret browsing disabled (accessibility.browsewithcaret=false)
 2. log into launchpad and go to the bug page and wait till the subscribers are loaded
 3. press the up and/or down key

(
Expected result: page scrolls
Actual result: page doesn't scroll as expected - feels like a missing focus.
)
 4. click on the page (i clicked in the title)
 5. use arrow keys

Expected result: page scrolls
Actual result: a caret appears which you can move on the document using the arrow keys.

 however, a caret appears and the arrow key move it through the document (which i refer to as caret browsing).

> 4. click on the page (i clicked in the title)
> 5. use arrow keys

Also, sometimes you dont need those two extra steps

(In reply to comment #8)
> that the preference is set), or just that the caret appears? Is the text
> editable?

no its not editable. you can just move the caret through the document using arrow keys.

This is still happening today (2009-08-07) on the main (deployed) Malone instance, eg. the URL:

  https://bugs.launchpad.net/malone/+bug/392138

exhibits the behaviour on about 50% of page loads.

Yes, just to confirm, the page *is* triggering the caret to appear in between the characters in the text, and that:

  accessibility.browsewithcaret=false

I reproduced this first time, by opening the above URL, PageDn, PageDn, PageDn (until at bottom), PageUp, PageUp, PageUp (until at top), and which point the PageUp/Down stopped working and the caret had appeared, movable via the Up/Down arrow keys.

Repeated tests to try to reproduce it (each started from a fresh page load):

1. Yes: PageDn (x3), PageUp (x3), page "locked", caret there
2. Yes: PageDn (hold), PageUp (hold), page locked, click for caret to appear
3. No.
4. Yes: PageUp (repeat), PageDn (repeat), PageUp (repeat), PageDown (repeat), Arrow Down, caret on
5. Yes: Hold Down Arrow whilst page loading, Page jumps back (almost) to top, page "locked", click for caret to show
6. Yes: Hold Down Arrow whilst page loading, scroll down, jump back to top, scolls down again, jump to top again, page locks; click to different tag, click back again, page unlocks.
7. Yes: Page locked itself, click for caret to appear, change tabs and back again, page unlocked.
8. Yes: Hold Arrow Down whilst page loading, page jumped + locked, change tabs and back to unlock.
9. Yes: Hold Up Arrow whilst page loading, page locked
10. Yes: Hold Page Down whilst page loading, page jumped to top and locked, click for caret to appear.

So, nine times out of ten, reproducibility. The most reliable way (although not the only way) appears to be to hold down one of the scrolling (Arrow or Paging) keys whilst the page is loading. And that changes tabs clears caret browsing mode.

The Downstream bug report has that produce cross-platform (and cross browser) so appears to be Gecko itself kicking into caret mode, rather than the browser interface enabling it.

Created attachment 565950
Fixed proof of concept

Comment on attachment 565950
Fixed proof of concept

Testing and debugging shows that this seems to work.

Neil, does your patch make the test part of attachment 548924 pass? If so, we can get them landed. :-)

(In reply to Ehsan Akhgari from comment #37)
> If so, we can get them landed. :-)

Except it's very much proof of concept (although I wouldn't mind landing the nsFocusManager.cpp change which would be necessary anyway).

In particular, I'm not sure a) which keys need to be changed b) whether those keys should map to the same commands in textareas and browsers/editors c) how native Linux key bindings fit in.

Created attachment 569554
Proposed patch

The names of the commands are configured in three places:
* GTK2 Native keybindings
* Editor bindings (editor-base.inc and libeditor)
* Browser bindings (browser-base.inc and nsGlobalWindowCommands)
The browser bindings have six differences to the other two sets of bindings,
all cmd_scroll* names, which I have renamed for consistency.
The browser bindings were missing implementations for selecting by page.
The stubs also used the wrong command names, so I corrected that too.
This then allowed me to remove the editor implementation.

Comment on attachment 569554
Proposed patch

Review of attachment 569554:
-----------------------------------------------------------------

Looks very good!

Does my test case pass with this patch?

(In reply to Ehsan Akhgari from comment #40)
> Does my test case pass with this patch?
Yes, it does.

So, things didn't go quite as well as I'd hoped.

Text areas and input fields need to have the editor behaviour.
Design mode and contenteditable windows need to have the global behaviour.

Created attachment 570250
Fixed patch

So, I ended up having to separate the cursor movement commands into their own command table so that I could append that controller for textareas and inputs.

Neil, any news about this bug?
It affects Fx9+(current beta), if we don't fix it in Fx11, then user have to wait at least 3*6=18 weeks before thay can copy text in those page again...

Sorry, I mistaken this bug for bug 692153.

(In reply to dindog from comment #44)
> Neil, any news about this bug?
ehsan, should I be asking someone else for review?

> It affects Fx9+(current beta), if we don't fix it in Fx11, then user have to
> wait at least 3*6=18 weeks before thay can copy text in those page again...

(In reply to dindog from comment #45)
> Sorry, I mistaken this bug for bug 692153.
Actually it turned out to be fairly straightforward to adapt this patch to apply to Copy, but it won't help with the Select All case.

Comment on attachment 570250
Fixed patch

Review of attachment 570250:
-----------------------------------------------------------------

I'm not crazy about "editor" vs. "editing" controllers, but I couldn't really think of anything better, so r=me.

Thanks a lot for your work on this, Neil, and sorry that I did a very bad job at responding to the review request in time.

Try run for e3859a3ea446 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e3859a3ea446
Results (out of 208 total builds):
    success: 144
    warnings: 50
    failure: 14
Builds (or logs if builds failed) available at:
http://<email address hidden>

test_showcaret.xul: uses cmd_scrollBottom, got renamed to cmd_moveBottom
test_selection_move_commands.xul: many renames...
test_movement_by_characters/word.html: rely on the buggy behaviour!

test_selection_move_commands actually tests for the broken commands. Sigh...

test_movement_by_characters.html's reliance on this bug is due to it failing to focus the content before performing the navigation.

test_movement_by_words.html has the above bug but did also find a real bug; with this patch, you can't move by words to the very end of a contenteditable area, because it tries to move right out of the contenteditable area...

Try run for 8c5b9300caf0 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=8c5b9300caf0
Results (out of 208 total builds):
    exception: 1
    success: 137
    warnings: 56
    failure: 14
Builds (or logs if builds failed) available at:
http://<email address hidden>

Created attachment 584062
With test fixes (for check in)

Try run for f0084cbf3b9d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f0084cbf3b9d
Results (out of 206 total builds):
    success: 167
    warnings: 24
    failure: 15
Builds (or logs if builds failed) available at:
http://<email address hidden>

> warnings: 24
> failure: 15
Most of these are hidden; 3 randomorange show up on tbpl by default.
More oranges and the reds show up with noignore or via self-serve but they're all Lion, Tegra and/or Jetpack so nobody cares about them.

This bug is fixed in 20121224 nightly. Neil, would you please look into the copy menuitem too?

Ehsan, sorry again for the other day urge you to review this bug in IRC. I got a reason to worried. Two major sites in China use contenteditable element in part of there service, (In fact, they rank No.1 & No.2 in China and No.5 & No.10 all over the world in Alexa), both are popular things, try imagining as they are Chinese Facebook and Google group, you'll got the picture.

I heard complaint about unable to copy and scroll after Fx9.0 was released. I don't want Firefox losing users. I'll would be better have it fixed not only in nightly, but also beta, even release.

*It* 'll would be better have it fixed not only in nightly, but also beta, even release.

sorry for the misspell

Pushed changeset 4d0391866459 to mozilla-central.

(In reply to dindog from comment #56)
> Neil, would you please look into the copy menuitem too?
Yes, I'll do copy (but not select all, because it's harder) in bug 692153.

"Dude, where's my commands?!" This change removed commands that have been around for many years, breaking add-ons: bug 714164.

Neil, I think you need to add those commands back, and just make us not use them... :(

Fair enough, and I can put the test back too, but I might as well use bug 714164.

*** Bug 702064 has been marked as a duplicate of this bug. ***

Bug 696020 appears to have exacerbated this problem in 10 (see bug 702064). The attached patch appears to be higher risk than we're comfortable for Beta (please correct me if I'm wrong), but I'm wondering if this is a good candidate for Aurora 11.

(In reply to Alex Keybl [:akeybl] from comment #63)
> Bug 696020 appears to have exacerbated this problem in 10 (see bug 702064).
> The attached patch appears to be higher risk than we're comfortable for Beta
> (please correct me if I'm wrong), but I'm wondering if this is a good
> candidate for Aurora 11.

I'd be much more comfortable if we could back out bug 696020 from Aurora. smaug, can we do that?

No. We were thinking to release 9.0.2 with bug 696020 fixed.

Or if we do that, we need to backout all
bug 696020, bug 689564 and bug 659350.

*** Bug 731924 has been marked as a duplicate of this bug. ***

If I could request a summary of the status, where are we on this bug now? dindog@163's saying that it's fixed in 20121224 nightly? How about in release? When is this going to be seen in a release, and when?

If I've done my sums right, you should find that it's currently available in Firefox Aurora, will become available in Beta on the 13th, and then released on the 24th of April. (If you think 17 weeks is a long time to wait, don't forget that Firefox 4 came out 14 months after Firefox 3.6!)

Woot, I see this now in Firefox 12. (This is the main reason we're still using a designMode iframe in G+)

(In reply to garryb from comment #70)
> Woot, I see this now in Firefox 12. (This is the main reason we're still
> using a designMode iframe in G+)

Great! Please file new bugs if you see more problems blocking you from moving to contenteditable elements.

As of 13.0.1 this still hasn't been fixed. Please see attached example:
http://pastebin.com/WTiitrQy

(In reply to Jakub Ondrusek from comment #72)

Please file a new Report in the Core/Editor Component with your Testcase attached. Thanks!