Closed Bug 440515 Opened 11 years ago Closed 11 years ago

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

Categories

(SeaMonkey :: Autocomplete, defect)

PowerPC
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: kairo, Assigned: neil)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Version: SeaMonkey 1.1 Branch → Trunk
Attached patch Potential patch (obsolete) — Splinter Review
Basically this needs Mac testing, so I'll take any two reviews ;-)
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #328063 - Flags: review?(stefanh)
Attachment #328063 - Flags: review?(jag)
Attachment #328063 - Flags: review?(iann_bugzilla)
Attachment #328063 - Flags: review?(bugzilla)
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
Attached patch Revised patchSplinter Review
This should now pass all the mochitests.
* Improved documentation
* Changed a return from false to true
Attachment #328063 - Attachment is obsolete: true
Attachment #328131 - Flags: review?(stefanh)
Attachment #328131 - Flags: review?(jag)
Attachment #328131 - Flags: review?(iann_bugzilla)
Attachment #328131 - Flags: review?(bugzilla)
Attachment #328063 - Flags: review?(stefanh)
Attachment #328063 - Flags: review?(jag)
Attachment #328063 - Flags: review?(iann_bugzilla)
Attachment #328063 - Flags: review?(bugzilla)
Comment on attachment 328131 [details] [diff] [review]
Revised patch

one down ;-) r=me.
Attachment #328131 - Flags: review?(bugzilla) → review+
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"?
Attachment #328131 - Flags: review?(stefanh) → review+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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?
(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.
Target Milestone: --- → seamonkey2.0alpha
> > 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...
Blocks: 443837
Comment on attachment 328131 [details] [diff] [review]
Revised patch

clearing seemingly obsolete review requests.
Attachment #328131 - Flags: review?(jag)
Attachment #328131 - Flags: review?(iann_bugzilla)
Patch checked into mozilla-central in Bug 443837
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/c188a0d1e01c
Allow up/down arrows to be home/end on the Mac r=Standard8,stefanh [p=Neil]
You need to log in before you can comment on or make changes to this bug.