I added support for the i, j, k, l keys in results.html, and they should scroll to the selected result. The scrolling no longer works. I can hear the baby jesus crying.
This broke in http://trac.webkit.org/changeset/122880 when the style was changed to body { display: -webkit-flex; }
Created attachment 155631 [details] Patch
Comment on attachment 155631 [details] Patch Why all this craziness, and not just a normal scrolling document?
Comment on attachment 155631 [details] Patch Sorry, didn't see Simon's comment.
(In reply to comment #3) > (From update of attachment 155631 [details]) > Why all this craziness, and not just a normal scrolling document? It's not actually that much craziness, is it? It's an extra container flexbox div and scrolling a div element instead of the window. The rest of the code changes are just fixing unrelated bugs I stumbled upon as I was testing this. It's so the flagged tests always show up at the bottom of the viewport. I was watching people over their shoulders using flagging and they didn't understand what flagging did because the text box was scrolled out of view. Also, it bugs me when I flag things and I need to scroll to get the text box.
Can't the flagging box just be position:fixed, bottom: 10px?
(In reply to comment #6) > Can't the flagging box just be position:fixed, bottom: 10px? Then it overlaps the content at the end of the page.
Comment on attachment 155631 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155631&action=review > LayoutTests/fast/harness/results.html:1121 > + var container = document.querySelector('.content-container'); > // rowRect is in client coords (i.e. relative to viewport), so we just want to add its top to the current scroll position. > - window.scrollTo(window.scrollX, window.scrollY + rowRect.top - 20); > + container.scrollTop += rowRect.top - 20; To be clear, this is the only part of this patch that is needed for fixing the scrolling. The rest is to make navigating/flagging unexpected passes work.
Comment on attachment 155631 [details] Patch Marking cq+. Simon, it doesn't seem like you object to this getting committed. If you do, feel free to cq- or rollout if this get committed before you notice.
I'm ok with it.
Comment on attachment 155631 [details] Patch Clearing flags on attachment: 155631 Committed r124287: <http://trac.webkit.org/changeset/124287>
All reviewed patches have been landed. Closing bug.