1278864 - jquery element.focus() does not bring element into view

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[soundobj]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160502160818

Steps to reproduce:

create a <div class="content" tabindex="-1">content goes here</div>
position page so that the div is outside of the viewport
programatically 
$(".content").focus();


Actual results:

the div is not scrolled into view


Expected results:

the div should be scrolled into view

Comment 1

[soundobj]

testing the same behaviour on Firefox version 45.1.1 works ok

Comment 2

[Loic]

Could you attach a testcase to the bug report and test with the latest version, v47., please.

Comment 3

[soundobj]

@loic I have tested with the latest version, hence the issue. I am just noting that reverting to an V45 cures the defect

Comment 4

[soundobj]

Attachment: testcase

I am unaware of the format for a testcase but here are the steps to reproduce

- create this markup <div class="testcase" tabindex="-1">some content here</div>
on a document
- position the newly created markup outside of the viewport
- programatically execute $(".testcase").viewport
EXPECTED:
the div is brought into viewport
ACTUAL:
thie div isn't brought into the viewport

Comment 5

[Loic]

Just attach a testcase, a full webpage in HTML (.html). Or you can use a website like http://codepen.io/ or https://jsfiddle.net/ to share a testcase.

Comment 6

[soundobj]

@Loic here is the codePen illustrating the issue: http://codepen.io/soundobj/pen/beEvdd

If you try chrome and click the 'bring to view' button it brings the whole div into view.
however if you try firefox it fails to bring the whole div. if you where to add more spacing between the button and the div (add more <br /> and the div is not visible in the viewport) and click 'bring into view' it will bring the whole div into view. so the issue arises when the element is partially visible. note that earlier versions of Firefox was working as expected.

Comment 7

[Kohei Yoshino [:kohei]]

focus() is not a method to bring an element into view. This is what you should use instead:
https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView

Comment 8

[soundobj]

@Kohei, I appreciate your feedback however there is an issue with using scrollIntoView() instead of $.focus()


- focus() scrolls only what is required to fully display the partially visible element into view; that has a huge impact in usability as the user context in the page remains. 
- scrollIntoView() brings the partially visible element to the top of the page. this has a big usability issue as the user context is greatly changed.

you can also see that if you completely hide the partially visible element need to be brought into view. focus() works fine. it is only when its partially visible that focus() does not work correctly. this was working ok on Firefox version 45.1.1

Looking forward hearing your thoughts.

Comment 9

[soundobj]

I have updated the pen with two buttons to illustrate my point you can test in Chrome and Firefox to see the differences, pls make the window the right size as to have the element needed to be focused partially visible.

Kind regards
David

Comment 10

[Loic]

Where is the new pen?

Comment 11

[soundobj]

http://codepen.io/soundobj/pen/beEvdd

Comment 12

[Loic]

(In reply to soundobj from comment #9)
> pls make the window the right
> size as to have the element needed to be focused partially visible.
 
I don't understand what you mean here. Could you explain better to reproduce the issue?

Comment 13

[Loic]

Ok, I tested again after reading again the entire bug report and I'm not able to reproduce:
*.focus(): if the div is partially visible in the viewport, scroll doesn't work. If it's not visible, it scrolls.
*.scrollIntoView(): in both cases, it scrolls.

I tested with FF47 and FF33 on Win 7, same result.

Comment 14

[soundobj]

@Loic Steps to reproduce. 

- enlarge the codepen so the two buttons are visible as well as partially the <div id="content"> so not all the content is visible and its hidden by the bottom end of the container
- click on 'focus' button
- expected: the full div is focused and all its contents are now visible so the hidden part of the div has been scrolled into the viewport.
-actual the div is focused but the hidden content has not been scrolled.

As a comparison when you click on the 'bring to view' button the <div id="content"> is scrolled to the top of the page.

If you try the same in other browsers (chrome or safari) you will see that clicking on 'focus' button scrolls the <div id="content"> so to show just its full content.

Note: this seems to be a regression as this was working ok on Firefox version 45.1.1

Comment 15

[Loic]

Thanks, I got it.

I can't prove it's a regression on Windows, as I said, focus() doesn't scroll to the div with old versions of Firefox like 33.

Boris, as you know DOM and you are on OSX, what do you think?

Comment 16

[Boris Zbarsky [:bzbarsky]]

I just tried the codepen in Firefox 45 on Mac.  It has the same behavior as tip when the focus button is clicked: the content is not scrolled.

Note that we should fix bug 1290152 to make it possible to produce the desired behavior with scrollIntoView.

Anyway, nsFocusManager::ScrollIntoView does a ScrollContentIntoView with SCROLL_MINIMUM as the "where to scroll", which should have the desired behavior if the scroll happened.  It also passes SCROLL_IF_NOT_VISIBLE as the "when to scroll".  This bug report seems to be asking for SCROLL_IF_NOT_FULLY_VISIBLE to be passed instead.

I suspect the problem is that using SCROLL_IF_NOT_FULLY_VISIBLE would mess up cases when the thing being focused is too big to fit in the scroll area: it would perform the scroll, and SCROLL_MINIMUM will scroll to top/left if the whole thing doesn't fit.  So it's not clear to me that we have a scroll mode that represents what the reporter is asking for, but I might be missing something....

Comment 17

[Boris Zbarsky [:bzbarsky]]

I put a testcase for the "thing being focused is larger than the visible area" at http://codepen.io/anon/pen/akKAPK

Comment 18

[Neil Deakin]

I think whatever flags 'nearest' would use would be correct. SCROLL_IF_NOT_FULLY_VISIBLE isn't correct when only the bottom is visible for example.