Last Comment Bug 325942 - [FIX] 'overflow: hidden' are scrolled by user action sometimes
: [FIX] 'overflow: hidden' are scrolled by user action sometimes
Status: NEW
: testcase
Product: Core
Classification: Components
Component: Layout: Misc Code (show other bugs)
: Trunk
: All All
: -- minor with 8 votes (vote)
: ---
Assigned To: Mats Palmgren (:mats)
:
Mentors:
http://www.leevigraham.com/holy_grail...
: 341254 345717 359575 433375 531057 533223 664275 771014 1211155 (view as bug list)
Depends on:
Blocks: 295020 259615 345717 359575
  Show dependency treegraph
 
Reported: 2006-02-04 18:20 PST by Mats Palmgren (:mats)
Modified: 2015-10-03 11:06 PDT (History)
20 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase #1, link target (2.22 KB, text/html)
2006-02-04 18:30 PST, Mats Palmgren (:mats)
no flags Details
Testcase #2, focus() (2.58 KB, text/html)
2006-02-04 18:31 PST, Mats Palmgren (:mats)
no flags Details
Testcase #3, unwanted parent view scrolling (1.64 KB, text/html)
2006-02-04 21:17 PST, Mats Palmgren (:mats)
no flags Details
Patch rev. 1 (7.11 KB, patch)
2006-02-05 17:42 PST, Mats Palmgren (:mats)
no flags Details | Diff | Review
Testcase 1b, link target (1.55 KB, text/html)
2006-06-08 16:09 PDT, Mats Palmgren (:mats)
no flags Details
Testcase 2b, focus() (1.81 KB, text/html)
2006-06-08 16:10 PDT, Mats Palmgren (:mats)
no flags Details
Testcase 3b, unwanted parent view scrolling (1.13 KB, text/html)
2006-06-08 16:11 PDT, Mats Palmgren (:mats)
no flags Details
Patch rev. 2 (5.48 KB, patch)
2006-06-08 16:23 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review

Description Mats Palmgren (:mats) 2006-02-04 18:20:02 PST
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 (:mats) 2006-02-04 18:30:52 PST
Created attachment 210729 [details]
Testcase #1, link target
Comment 2 Mats Palmgren (:mats) 2006-02-04 18:31:29 PST
Created attachment 210730 [details]
Testcase #2, focus()
Comment 3 Mats Palmgren (:mats) 2006-02-04 21:17:37 PST
Created attachment 210736 [details]
Testcase #3, unwanted parent view scrolling
Comment 4 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-02-05 13:35:25 PST
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) 2006-02-05 15:57:24 PST
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 (:mats) 2006-02-05 17:42:40 PST
Created attachment 210812 [details] [diff] [review]
Patch rev. 1

FWIW, this fixes all the attached testcases and the URL.
Comment 7 Mats Palmgren (:mats) 2006-02-05 17:44:22 PST
(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-7 (review requests must explain patch) 2006-02-05 18:31:54 PST
(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-7 (review requests must explain patch) 2006-02-06 09:59:37 PST
+    // 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 [:mwargers] (gone per 2016-05-31 :-( ) 2006-02-07 03:02:59 PST
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) 2006-06-02 18:15:10 PDT
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-7 (review requests must explain patch) 2006-06-02 18:26:17 PDT
Mats, were you planning to request reviews?
Comment 13 Mats Palmgren (:mats) 2006-06-08 16:09:27 PDT
Created attachment 224927 [details]
Testcase 1b, link target
Comment 14 Mats Palmgren (:mats) 2006-06-08 16:10:12 PDT
Created attachment 224928 [details]
Testcase 2b, focus()
Comment 15 Mats Palmgren (:mats) 2006-06-08 16:11:28 PDT
Created attachment 224929 [details]
Testcase 3b, unwanted parent view scrolling
Comment 16 Mats Palmgren (:mats) 2006-06-08 16:13:51 PDT
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 (:mats) 2006-06-08 16:18:14 PDT
(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 (:mats) 2006-06-08 16:21:10 PDT
(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 (:mats) 2006-06-08 16:23:33 PDT
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.
Comment 20 Mats Palmgren (:mats) 2006-06-08 16:36:15 PDT
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-7 (review requests must explain patch) 2006-06-10 16:17:50 PDT
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 (:mats) 2006-06-12 21:11:30 PDT
*** Bug 341254 has been marked as a duplicate of this bug. ***
Comment 23 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-03 23:31:48 PST
*** Bug 345717 has been marked as a duplicate of this bug. ***
Comment 24 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-03 23:33:31 PST
*** Bug 433375 has been marked as a duplicate of this bug. ***
Comment 25 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-03 23:34:12 PST
*** Bug 359575 has been marked as a duplicate of this bug. ***
Comment 26 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-03 23:34:51 PST
*** Bug 531057 has been marked as a duplicate of this bug. ***
Comment 27 XtC4UaLL [:xtc4uall] 2009-12-08 04:50:46 PST
*** Bug 533223 has been marked as a duplicate of this bug. ***
Comment 28 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-06-14 14:20:42 PDT
*** Bug 664275 has been marked as a duplicate of this bug. ***
Comment 29 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-07-06 14:20:16 PDT
*** Bug 771014 has been marked as a duplicate of this bug. ***
Comment 30 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-09-04 18:03:52 PDT
bug 674477 may have improved things here (probably not completely fixed, though)
Comment 31 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2015-10-03 11:06:18 PDT
*** Bug 1211155 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.