440515 - port bug 231754 (up/down arrow handling on mac) to xpfe autocomplete

Status: RESOLVED FIXED

(SeaMonkey :: Autocomplete, defect)

Description

[Robert Kaiser]

When investigating test failures for bug 436349, Mark found out that bug 231754, "Mac: pressing up/down arrow key does not move caret to beginning/end of url bar or text box", has apparently only been fixed for toolkit autocomplete and not xpfe, so tests for that fail on SeaMonkey.

Comment 1

[[e-mail]]

Attachment: Potential patch

Basically this needs Mac testing, so I'll take any two reviews ;-)

Comment 2

[Mark Banner (:standard8)]

Well its better ;-) out of the 7 previous failures I now see only 3:

*** 423 ERROR FAIL | no value up keypress event not fired | got true, expected false | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_autocomplete.xul
*** 427 ERROR FAIL | no value down keypress event not fired | got true, expected false | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_autocomplete.xul
*** 442 ERROR FAIL | value down with caret in middle again keypress event not fired | got true, expected false | chrome://mochikit/content/chrome/toolkit/content/tests/chrome/test_autocomplete.xul

Comment 3

[[e-mail]]

Attachment: Revised patch

This should now pass all the mochitests.
* Improved documentation
* Changed a return from false to true

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 328131 [details] [diff] [review]
Revised patch

one down ;-) r=me.

Comment 5

[Stefan [:stefanh]]

Comment on attachment 328131 [details] [diff] [review]
Revised patch

Works fine. I have not checked if the tests pass, though.

One small thing:
>+              // For compatibility for toolkit we now have to predict which

"For compatibility with toolkit"?

Comment 6

[[e-mail]]

Fix checked in.

Comment 7

[Robert Kaiser]

Looks good on the unit test box, which now succeeds on those tests.

Could you land in mozilla-central as well so we don't lose the fix when switching to Mercurial?

Comment 8

[[e-mail]]

(In reply to comment #7)
> Looks good on the unit test box, which now succeeds on those tests.
You confused me, becuase it was chrome tests that I was fixing ;-)

> Could you land in mozilla-central as well so we don't lose the fix when
> switching to Mercurial?
Bah, first editor/ui, and now this... btw I haven't installed even hg yet.

Comment 9

[Stefan [:stefanh]]

> > Could you land in mozilla-central as well so we don't lose the fix when
> > switching to Mercurial?
> Bah, first editor/ui, and now this... btw I haven't installed even hg yet.
> 
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/xpfe/components/autocomplete/resources/content/autocomplete.xml&root=/cvsroot versus http://hg.mozilla.org/mozilla-central/?log/tip/xpfe/components/autocomplete/resources/content/autocomplete.xml 

--> rev 1.165-1.168 of the cvs trunk version is missing in the mozilla-central file...

Comment 10

[Justin Wood (:Callek)]

Comment on attachment 328131 [details] [diff] [review]
Revised patch

clearing seemingly obsolete review requests.

Comment 11

[Justin Wood (:Callek)]

Patch checked into mozilla-central in Bug 443837

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c188a0d1e01c
Allow up/down arrows to be home/end on the Mac r=Standard8,stefanh [p=Neil]