Closed Bug 559499 Opened 14 years ago Closed 14 years ago

relatively positioned block element with z-index: -1 makes anchor tags in it unclickable

Categories

(Core :: Web Painting, defect)

Product:
Core
Component:
Web Painting
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- -
status2.0 --- wanted

People

(Reporter: mcbain.asm, Assigned: roc)

Details

(Keywords: testcase)

Attachments

(2 files, 1 obsolete file)

Testcase
208 bytes, text/html
Details
better fix
6.54 KB, patch
MatsPalmgren_bugz
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3

A default block element (such as a div) containing anchor tags when styled with z-index: -1 and position: relative renders the anchors in it unclickable. No hand cursor appears when the mouse rolls over the links either. The block element is not hiding behind anything as styling the html tag with background-color: yellow does not obscure the block.


Sample page:
<!DOCTYPE html>
<html style="background-color: yellow">
	<head><title>Test</title></head>
	<body style="position: relative; z-index: -1">
		<a href="http://mozilla.org">Mozilla</a>
	</body>
</html>

Reproducible: Always

Steps to Reproduce:
1. Save the sample page listed in the details to an HTML file.
2. Load it up in Firefox 3.6.3
3. Try to click the "Mozilla" link.
Actual Results:  
The "Mozilla" link is not clickable and does not show the hand cursor when the mouse is over the link.

Expected Results:  
The cursor should turn into a "hand" when it is over the link, and the link should be clickable.

The sample was tested in Firefox on Linux x86-64, and the link is not clickable there either. To ensure this was not expected behavior, the link in the sample was found to be clickable in both IE8 and Safari 4.0.5.
Attached file Testcase — 

    
    

    
  
Indeed this fails with Firefox 3, 3.5, 3.6 and latest trunk.
With Firefox 2 the link works, although the link is not visible.

    
    

    
  
If the positioning is fixed the link becomes active again, as soon as it is below the lowest element. Even if it is uncovered.

To chek or reproduce:
http://hirt-fotografie.ch

1.Open webpage and control if you can see "metasonix 2009" in the lower right corner of your browser. If not, widen the window (around 1100px)

2.If "metasonix 2009" is on the same hight as the lighter brown (the actual content) it is unclickable.

3.Resize your browser in a manner so that "metasonix 2009" drops below the content and it becomes clickable. (Easiest to do on the "Kontakt" page, depending on screen resolution)

The div containing the link has z-index:-1; and is nested within <body>. Nothing covering it, except the content if properly resized.

    
    

    
  
Display list log:

Event handling --- (2340,1020):
Background 0x7f43415abb08(HTMLScroll(html)(-1)) (0,0,55620,57240) uniform
Clip (nil)() (0,0,55620,57240)
    CanvasBackground 0x7f43415ab8c8(Canvas(html)(-1)) (0,0,55620,57240) uniform
Clip 0x7f43415c4620(Block(body)(1)) (0,0,55620,57240)
    WrapList 0x7f43415c4620(Block(body)(1)) (480,480,54660,1140)
        Background 0x7f43415c4620(Block(body)(1)) (480,480,54660,1140) uniform
        Background 0x7f43415c4b60(Inline(a)(1)) (480,480,3600,1140) uniform
        TextDecoration 0x7f43415c4b60(Inline(a)(1)) (480,480,3600,1140)
        Text 0x7f43415c4bc8(Text(0)) (480,480,3600,1140)
Clip (nil)() (0,0,55620,57240)
    Background 0x7f43415c40c8(Block(html)(-1)) (0,0,55620,2100) uniform

That last Background item doesn't look right to me....

    
    

    
  
nsFrame::DisplayBackgroundUnconditional adds the item if IsForEventDelivery().  roc, do you know why?
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true

    
    

    
  
Because you can always click on the background of an element even if it's fully transparent.

The problem here is that the background of the root element is going to the wrong place...

    
    

    
  
Well, nsFrame::DisplayBackgroundUnconditional says it doesn't do background propagation.... should it?

    
    

    
  
Oopsie. Here's what's happening:
-- the real background is CanvasBackground added by nsCanvasFrame
-- but the nsBlockFrame for the root element also calls DisplayBackgroundBorderOutline, whichlso adds an nsDisplayBackground display item (where the block background would go in the z-order)
-- That background doesn't paint anything, because nsCSSRendering::PaintBackground bails out when it finds the background was propagated away
-- But of course, we don't check that in nsDisplayBackground::HitTest, we just return the block

Probably the best fix is to change nsDisplayBackground::HitTest to check for background propagation.

    
    

    
  
Actually I need to look closer ... the html Background item should have been just above the CanvasBackground item, not later

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix (obsolete)
        — 
          — Splinter Review
      

    


  
This patch takes the approach of suppressing background displayitems for the root element where the real background is at the nsCanvasFrame/nsRootBoxFrame.
Assignee: nobody → roc

        Attachment #441717 -
        Flags: review?(matspal)

    
    

    
  
Do we need similar changes to propagated <body> backgrounds?

    
    

    
  
No, we don't.

Generally, an element that's a stacking context puts the backgrounds of its frame(s) at the bottom of the stacking context by collecting those backgrounds in its BorderBackgrounds list and then explicitly putting those at the bottom of its stacking context. That works fine for <body>. It doesn't work for the root element because there the stacking context root is the viewport frame and the not-really-a-background nsDisplayBackground for the block frame (or table frame) of the root element does not end up on the BorderBackgrounds list that is delivered to the viewport frame. And that's because nsCanvasFrame::BuildDisplayList forces its child block (or table) frame to be the root of a pseudo-stacking context.

I don't honestly remember why it had to do that. Maybe I should take that out and see what happens.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        better fix
        — 
          — Splinter Review
      

    


  
I can't remember why we were using a pseudo-stacking-context here. This patch passed reftests locally. This could change the z-ordering of page content relative to the viewport scrollbars, but we clip everything to the viewport scrollport anyway, so that shouldn't matter.

        Attachment #441717 -
        Attachment is obsolete: true

        Attachment #442039 -
        Flags: review?(matspal)

        Attachment #441717 -
        Flags: review?(matspal)

    
    

    
  
Comment on attachment 442039 [details] [diff] [review]
better fix

This patch is incomplete; I'm assuming the changes to nsDisplayList.*
from the first patch should have been included.

>layout/base/nsDisplayList.* in the first patch

ok

>layout/base/tests/Makefile.in

The current indentation is intentional to associate the files with
test_reftests_with_caret.html which runs them as reftests.

>layout/generic/nsCanvasFrame.cpp
>     rv = BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists,
>-                                  DISPLAY_CHILD_FORCE_PSEUDO_STACKING_CONTEXT);
>+                                  0);

I would prefer removing the last arg (the default is zero).

r=mats

        Attachment #442039 -
        Flags: review?(matspal) → review+
Keywords: testcase
OS: Windows XP → All
Hardware: x86 → All

    
    

    
  
(In reply to comment #14)
> (From update of attachment 442039 [details] [diff] [review])
> This patch is incomplete; I'm assuming the changes to nsDisplayList.*
> from the first patch should have been included.

No, in fact the changes to table code are not wanted. Can you confirm your r+ applies without that?

> >layout/base/tests/Makefile.in
> 
> The current indentation is intentional to associate the files with
> test_reftests_with_caret.html which runs them as reftests.

OK, I'll remove that.

> >layout/generic/nsCanvasFrame.cpp
> >     rv = BuildDisplayListForChild(aBuilder, kid, aDirtyRect, aLists,
> >-                                  DISPLAY_CHILD_FORCE_PSEUDO_STACKING_CONTEXT);
> >+                                  0);
> 
> I would prefer removing the last arg (the default is zero).

OK.

    
    

    
  
I'm confused, the change in nsFrame.cpp also requires ShouldDisplayBackground.
Are you saying the only code change is the one in nsCanvasFrame.cpp?

    
    

    
  
ok, r=mats on that
Whiteboard: [needs landing]
blocking2.0: ? → -

          status2.0:
          --- → wanted

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/40fe9cebcab5
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]

    
    

    
  
Please fill the same bug to the Opera ts. Opera10.5 - get it too.
Component: Layout: View Rendering → Layout: Web Painting

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: