Open Bug 325942 Opened 14 years ago Updated 1 year ago

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

Categories

(Core :: Layout, defect, minor)

defect
Not set
minor

Tracking

()

People

(Reporter: mats, Assigned: mats)

References

(Blocks 1 open bug, )

Details

(Keywords: testcase)

Attachments

(7 files, 1 obsolete file)

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.
Attached file Testcase #2, focus()
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.
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?).
Attached patch Patch rev. 1 (obsolete) — Splinter Review
FWIW, this fixes all the attached testcases and the URL.
(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...
(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.

+    // 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?
Just wondering, with the patch, does the acid2 test still work? 
http://www.webstandards.org/act/acid2/test.html
Severity: minor → enhancement
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.
Severity: enhancement → minor
Mats, were you planning to request reviews?
Attached file Testcase 2b, focus()
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.
(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.
(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).
Attached patch Patch rev. 2Splinter Review
This patch keeps the scrolling mostly as is, but it fixes the URL and does
not break acid2.
Attachment #210812 - Attachment is obsolete: true
Attachment #224933 - Flags: review?(dbaron)
Summary: 'overflow: hidden' are scrolled by user action sometimes → [FIX] 'overflow: hidden' are scrolled by user action sometimes
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.
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.
*** Bug 341254 has been marked as a duplicate of this bug. ***
Attachment #224933 - Flags: review?(dbaron)
Blocks: 345717
Blocks: 359575
Duplicate of this bug: 345717
Duplicate of this bug: 433375
Duplicate of this bug: 359575
Duplicate of this bug: 531057
Duplicate of this bug: 533223
Duplicate of this bug: 664275
Duplicate of this bug: 771014
bug 674477 may have improved things here (probably not completely fixed, though)
Duplicate of this bug: 1211155
See Also: → 1272084
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.