Community
Participate
Working Groups
Build Identifier: Cross-posting from https://bugzilla.mozilla.org/show_bug.cgi?id=687573 Steps to reproduce: 1. Paste this into scratch pad: XPCOMUtils.defineLazyGetter(window, "gFindBar", function() { let XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"; let findbar = document.createElementNS(XULNS, "findbar"); findbar.id = "FindToolbar"; let browserBottomBox = document.getElementById("browser-bottombox"); browserBottomBox.insertBefore(findbar, browserBottomBox.firstChild); // Force a style flush to ensure that our binding is attached. findbar.clientTop; findbar.browser = gBrowser; window.gFindBarInitialized = true; return findbar; }); Make sure that the width of the scratchpad window is small enough for there to be a vertical scrollbar. 2. Put the caret at the end of the second line by scrolling the view horizontally. 3. Press down 4 times. The caret should end up at the end of the "let browserBottomBox" line... (I reproduced this just now by ensuring that I had both horizontal and vertical scrollbars). Reproducible: Always
I have a fix ready to review for this bug in my git clone at orion.eclipse.org. Not sure how I can export the local branch to others from the team? Without pushing to the main Orion git repo.
So, I found a way to push from orion.eclipse.org to my github repo clone. Here is the branch with the fix: https://github.com/mihaisucan/orion.client/tree/moz-bug-687573 Fix explanation: the _columnX property is relative to the Orion view and it tells the X offset from the left edge of the iframe, without taking scrolling into account. When the user scrolls the code needs to take into account the vertical scroll difference and compensate the _columnX offset value. The same problem applies to the Page Up / Down keys (not just the Up/Down arrow keys). However, for Page Up / Down we also need to make sure the caret is visible after scrolling. (the call to _scrollView() only made sure that the line is visible, without checking if the caret is also visible) Comments are very much welcome! Please review and let me know if further changes are needed. Thank you!
(In reply to comment #2) > Comments are very much welcome! Please review and let me know if further > changes are needed. Thank you! Hi Mihai, I think your solution works. Note that columnX also comes from StyledText. See StyledText#doLineUp for example. There we also fix columnX by the horizontal scroll delta caused by the operation. The difference is that everything is synchronous, what makes the code easier. What I was thinking for TextView was to make columnX relative to the content (so scrolling does not changing columnX). I think that would be easier. I will try it out.
Hello Felipe! (In reply to comment #3) > (In reply to comment #2) > > Comments are very much welcome! Please review and let me know if further > > changes are needed. Thank you! > > Hi Mihai, I think your solution works. > > Note that columnX also comes from StyledText. See StyledText#doLineUp for > example. There we also fix columnX by the horizontal scroll delta caused by the > operation. > The difference is that everything is synchronous, what makes the code easier. > > What I was thinking for TextView was to make columnX relative to the content > (so scrolling does not changing columnX). I think that would be easier. I didn't look into StyledText, actually I just learned about it. ;) I tried to use a content-relative columnX, but I did bump into different issues. YMMV. > I will try it out. Thank you for looking into the code! Please let me know if further changes are needed from me.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=10ae999ff6bf24d31a431bd6c34f08ec64aa2270(In reply to comment #4) > I didn't look into StyledText, actually I just learned about it. ;) > > I tried to use a content-relative columnX, but I did bump into different > issues. YMMV. > It worked for me, can you pull the latest and give it try ? Thank you
(In reply to comment #5) > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=10ae999ff6bf24d31a431bd6c34f08ec64aa2270(In > reply to comment #4) > > > I didn't look into StyledText, actually I just learned about it. ;) > > > > I tried to use a content-relative columnX, but I did bump into different > > issues. YMMV. > > > > It worked for me, can you pull the latest and give it try ? > Thank you Hah! When I tried the content-relative coordinates it failed to work because I was affected by bug 360724. Only much later I noticed the different problem. Thank you for the fix! But you did not include x scrolling for Page Up/Down. Orion should make the caret visible when you do page up/down. In my patch I included logic for making sure the caret is visible. Is this behavior not desired? I tested other editors and they do make the caret visible.
(In reply to comment #6) > But you did not include x scrolling for Page Up/Down. Orion should make the > caret visible when you do page up/down. In my patch I included logic for making > sure the caret is visible. Is this behavior not desired? I tested other > editors and they do make the caret visible. Yes, the caret should always be visible after page_down/page_up. Hopefully I got this fixed in Bug 358615. Please, test it and let me know if it working for you. Thank you.
(In reply to comment #7) > (In reply to comment #6) > > But you did not include x scrolling for Page Up/Down. Orion should make the > > caret visible when you do page up/down. In my patch I included logic for making > > sure the caret is visible. Is this behavior not desired? I tested other > > editors and they do make the caret visible. > > Yes, the caret should always be visible after page_down/page_up. > Hopefully I got this fixed in Bug 358615. > > Please, test it and let me know if it working for you. Thank you. Both fixes are confirmed here as well. Thank you!
