325942 - [FIX] 'overflow: hidden' are scrolled by user action sometimes

Status: RESOLVED WONTFIX

(Core :: Layout, defect)

Description

[Mats Palmgren (inactive)]

Spawned off from bug 259615 comment 28

A overflow:hidden view is scrolled when trying to make an element visible
because it received focus or is a link target of a fragment URI.

STEPS TO REPRODUCE
1. load url
2. scroll to the end of the page
3. click on the link

ACTUAL RESULT
The grey area is scrolled so that most of the content becomes unviewable
(and impossible to scroll into view again).

EXPECTED RESULT
The grey area should not scroll when trying to scroll the link target
into view. If "Target Of Link" is outside the visible content area,
the viewport should scroll to make it visible.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Testcase #1, link target

Comment 2

[Mats Palmgren (inactive)]

Attachment: Testcase #2, focus()

Comment 3

[Mats Palmgren (inactive)]

Attachment: Testcase #3, unwanted parent view scrolling

Comment 4

[Martijn Wargers (dead)]

IE6 also scrolls the overflow:hidden view when trying to make an element visible
because it received focus or is a link target of a fragment URI.

Comment 5

[philippe (part-time)]

Opera 9 (TP1) works exactly the same as Gecko in this (they changed the behaviour with this build).

Note that the description of overflow:hidden changed between CSS2.0 and CSS2.1, allowing for this behaviour. Members of the CSS WG might explain this better (David , or Ian?).

Comment 6

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

FWIW, this fixes all the attached testcases and the URL.

Comment 7

[Mats Palmgren (inactive)]

(In reply to comment #4)
> IE6 also scrolls the overflow:hidden view ...

Ok, maybe we want to be compatible and allow scrolling of overflow:hidden
in this case. I still think our behaviour for the URL is unreasonable - since
there is a surrounding view (the viewport) that can scroll the link target
into view without causing content to be clipped this should be preferred.
The "unwanted parent view scrolling" is a bug IMHO, I'll see if I can fix those
things but still allow overflow:hidden scrolling...

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

(In reply to comment #4)
> IE6 also scrolls the overflow:hidden view when trying to make an element
> visible
> because it received focus or is a link target of a fragment URI.

I still think we shouldn't scroll.

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

+    // If the frame is completely clipped by this view then scrolling
+    // surrounding views isn't going to make it visible. (bug 325942)
do we really want to do this?  Should we instead just scroll to where it would be if it weren't clipped?

Also, while you're here, could you fix the frame type check for inlineFrame to also check for positionedInlineFrame?

Comment 10

[Martijn Wargers (dead)]

Just wondering, with the patch, does the acid2 test still work? 
http://www.webstandards.org/act/acid2/test.html

Comment 11

[Caio Tiago Oliveira (asrail)]

Sorry for changing the severity, it was a shortcut to access the preferences dialog while in this page (alt+e e) and I haven't noticed it before submitting.

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Mats, were you planning to request reviews?

Comment 13

[Mats Palmgren (inactive)]

Attachment: Testcase 1b, link target

Comment 14

[Mats Palmgren (inactive)]

Attachment: Testcase 2b, focus()

Comment 15

[Mats Palmgren (inactive)]

Attachment: Testcase 3b, unwanted parent view scrolling

Comment 16

[Mats Palmgren (inactive)]

IE7-beta2 on XPSP2:
  Testcase1b, 1:  FAIL (top-of-view)
  Testcase1b, 2:  PASS (top-of-view)
  Testcase1b, 3:  PASS (top-of-view)
  Testcase1b, 4:  FAIL (top-of-view)

  Testcase2b, 1:  FAIL (no-viewport-scroll)
  Testcase2b, 2:  PASS (no-viewport-scroll)
  Testcase2b, 3:  PASS (no-viewport-scroll)
  Testcase2b, 4:  FAIL (no-viewport-scroll)

  Testcase3b, 1:  FAIL (no-viewport-scroll)
  URL: ok, scrolls target to top-of-view and no clip

Opera8.01 Linux:
  Testcase1b, 1:   -   (end-of-document)
  Testcase1b, 2:  PASS (scrolls viewport slightly but not top-of-view)
  Testcase1b, 3:  PASS (top-of-view)
  Testcase1b, 4:   -   (end-of-document)

  Testcase2b, 1:   -   (end-of-document)
  Testcase2b, 2:  PASS (no-viewport-scroll)
  Testcase2b, 3:  PASS (no-viewport-scroll)
  Testcase2b, 4:   -   (end-of-document)

  Testcase3b, 1:  FAIL2 (no-viewport-scroll)
  URL: ok, scrolls target to top-of-view and no clip

Opera9-beta2 Linux:
  Testcase1b, 1:   -   (end-of-document)
  Testcase1b, 2:  PASS (scrolls viewport wrong, PASS is above)
  Testcase1b, 3:  PASS (top-of-view)
  Testcase1b, 4:   -   (end-of-document)

  Testcase2b, 1:   -   (end-of-document)
  Testcase2b, 2:  PASS (no-viewport-scroll)
  Testcase2b, 3:  PASS (no-viewport-scroll)
  Testcase2b, 4:   -   (end-of-document)

  Testcase3b, 1:   -   (end-of-document)
  URL: ok, scrolls target to top-of-view and no clip

Mozilla trunk Linux:
  Same result as IE7-beta2, except for URL (this bug).

Mozilla with "Patch rev.1" Linux:
  Testcase1b, 1:   -   (no-viewport-scroll)
  Testcase1b, 2:  PASS (top-of-view)
  Testcase1b, 3:  PASS (top-of-view)
  Testcase1b, 4:   -   (no-viewport-scroll)

  Testcase2b, 1:   -   (no-viewport-scroll)
  Testcase2b, 2:  PASS (top-of-view)
  Testcase2b, 3:  PASS (top-of-view)
  Testcase2b, 4:   -   (no-viewport-scroll)

  Testcase3b, 1:   -   (no-viewport-scroll)
  URL: ok, scrolls target to top-of-view and no clip
  ACID2: fails.

Mozilla with "Patch rev.2" Linux:
  Same result as IE7-beta2. ACID2: ok.

Comment 17

[Mats Palmgren (inactive)]

(In reply to comment #10)
> Just wondering, with the patch, does the acid2 test still work? 
> http://www.webstandards.org/act/acid2/test.html

Good catch, it actually fails with Patch rev.1 since the acid2 test depends
on the UA to scroll an overflow:hidden view when trying to make an element
visible because it is a link target of a fragment URI.

Comment 18

[Mats Palmgren (inactive)]

(In reply to comment #5)
> Opera 9 (TP1) works exactly the same as Gecko in this (they changed the
> behaviour with this build).

This is not what I'm seeing with Opera9-beta2 on Linux (see comment 16).

Comment 19

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

This patch keeps the scrolling mostly as is, but it fixes the URL and does
not break acid2.

Comment 20

[Mats Palmgren (inactive)]

I can probably make a new version of rev.1 that doesn't screw up acid2
if that's is the way we want to go.  rev.1 is mostly compatible with
Opera9b2/Linux and rev.2 is compatible with IE7b2/XP. I would guess that
there is some risc for regressions with rev.1 though.

Comment 21

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think one of the things we need to do here is distinguish user action from author action; in other words, navigating to a link target should be allowed to scroll things that are 'overflow: hidden', but focusing (and selection, but I *think* that's already fixed) should not.

It looks like what you're trying to fix here is that we scroll an outer container even when something is clipped by an inner one; that seems like a good thing to fix as well.  But I'd appreciate if you could explain the patch a little bit more when asking for review.

Comment 22

[Mats Palmgren (inactive)]

*** Bug 341254 has been marked as a duplicate of this bug. ***

Comment 30

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

bug 674477 may have improved things here (probably not completely fixed, though)

Comment 32

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S4. However, the bug has 9 duplicates.
:dholbert, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 34

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 35

[Daniel Holbert [:dholbert]]

FWIW: I clicked through the links/buttons in all of the testcases here, and our behavior exactly matches Chrome (our "pass"/"fail" results are all precisely the same as Chrome's).

I think expectations/intent were a bit different here based on different spec & interop situation 17 years ago when this was filed.

Today 17 years later, we have perfect interop with our primary competitor on the attached testcases, so it's not obvious that there's a good reason for us to change behavior here. Specs have evolved quite a bit over the last 17 years as well; I suspect our interop here is spec-aligned -- though if it turned out to not be, we can take that up in a new bug or spec issue.