Summary: | i and j keys no longer scroll in results.html | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||
Component: | Tools / Tests | Assignee: | Ojan Vafai <ojan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | dpranke, ojan, simon.fraser, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2012-07-28 14:55:07 PDT
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. |