The default bug view has changed. See this FAQ.

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

NEW
Assigned to

Status

()

Core
Layout: Misc Code
--
minor
11 years ago
7 months ago

People

(Reporter: mats, Assigned: mats)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(7 attachments, 1 obsolete attachment)

(Assignee)

Description

11 years ago
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.
(Assignee)

Comment 1

11 years ago
Created attachment 210729 [details]
Testcase #1, link target
(Assignee)

Comment 2

11 years ago
Created attachment 210730 [details]
Testcase #2, focus()
(Assignee)

Comment 3

11 years ago
Created attachment 210736 [details]
Testcase #3, unwanted parent view scrolling
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

11 years ago
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?).
(Assignee)

Comment 6

11 years ago
Created attachment 210812 [details] [diff] [review]
Patch rev. 1

FWIW, this fixes all the attached testcases and the URL.
(Assignee)

Comment 7

11 years ago
(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?
(Assignee)

Comment 13

11 years ago
Created attachment 224927 [details]
Testcase 1b, link target
(Assignee)

Comment 14

11 years ago
Created attachment 224928 [details]
Testcase 2b, focus()
(Assignee)

Comment 15

11 years ago
Created attachment 224929 [details]
Testcase 3b, unwanted parent view scrolling
(Assignee)

Comment 16

11 years ago
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.
(Assignee)

Comment 17

11 years ago
(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.
(Assignee)

Comment 18

11 years ago
(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).
(Assignee)

Comment 19

11 years ago
Created attachment 224933 [details] [diff] [review]
Patch rev. 2

This patch keeps the scrolling mostly as is, but it fixes the URL and does
not break acid2.
Attachment #210812 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Attachment #224933 - Flags: review?(dbaron)
(Assignee)

Updated

11 years ago
Summary: 'overflow: hidden' are scrolled by user action sometimes → [FIX] 'overflow: hidden' are scrolled by user action sometimes
(Assignee)

Comment 20

11 years ago
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.
(Assignee)

Comment 22

11 years ago
*** Bug 341254 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

11 years ago
Attachment #224933 - Flags: review?(dbaron)

Updated

11 years ago
Blocks: 345717
(Assignee)

Updated

9 years ago
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
Blocks: 295020
Duplicate of this bug: 771014
bug 674477 may have improved things here (probably not completely fixed, though)
Duplicate of this bug: 1211155

Updated

7 months ago
See Also: → bug 1272084
You need to log in before you can comment on or make changes to this bug.