Closed Bug 541406 Opened 15 years ago Closed 14 years ago

Firefox 3.6 regression: plugins get a focus outline (which causes scrollbars)

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Version:
1.9.2 Branch
Platform:
x86
Windows XP
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- -
status1.9.2 --- .2-fixed

People

(Reporter: patrick, Assigned: roc)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

fix v2
1.45 KB, patch
roc
: review+
beltzner
: approval1.9.2.2+
Details | Diff | Splinter Review
revert (and add embed)
1.97 KB, patch
roc
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.9.2) Gecko/20100115 Firefox/3.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.9.2) Gecko/20100115 Firefox/3.6

When visiting a site which displays a java applet occupying 100% of the window. Firefox displays scrollbars when it has focus. If firefox looses focus the rendering of the scrollbars is no longer done.

I expect no rendering of scrollbars in this case (whether firefox has focus or not).

Further I notice a lot of screen flashing when displaying a Java applet, which I think is undesirable.

Reproducible: Always

Steps to Reproduce:
1. Visit http:cecile.nu (an example of a site with a Java applet with 100% width and height)

Actual Results:  
Scrollbars are displayed and screen flashes a lot.

Expected Results:  
I expect no scrollbars and no flashing of the screen.
Component: General → Plug-ins
Product: Firefox → Core
QA Contact: general → plugins
Version: unspecified → 1.9.2 Branch
Confirmed, for Windows Vista and Firefox 3.6.
I encountered it on http://www.runescape.com/game.ws
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
blocking1.9.2: --- → ?
blocking2.0: --- → ?
I have the same problem on http://www.runescape.com/game.ws
Also i have more info i tried to debug it with firebug and it seams the problem stops if scrolling="auto" is changed to scrolling="no"
Martijn, do you think you can hunt down the regression range here?
could u reword that? i don't understand what ur after
This bug is flagged a regression.  That is, it used to work and now doesn't.  Is that correct? If so, which version did it work in?

And more to the point, on what exact day did it break?  There are daily builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009/ month by month (and replace 2009 by 2008 to find earlier ones).
What happens is when the user upgrades from ff 3.5 to 3.6 the website is read improperly or written wrong and causes the scroll bars to come up.

Note:That is all tht needs to happen the java version as far as i can tell doesn't matter.
OK.  So this worked in Firefox 3.5.  Now someone needs to find when the behavior changed.  In comment 4 I asked Martijn to do that, but if you have the time to do so instead, that's great.  If you don't, that's fine too.
srry i can't help u i'm a freshman in hs nt really sure how to go about it but hey i'm trying =)
srry tht i can't help
http://www.runescape.com/game.ws

This is the site where I discovered this too. Strange thing is: if you switch to another window, like chat box for icq or whatever, the scroll bars disappear and appear again at the moment you change back to fireFox.

This was never a problem with 3.5 and always occurs to me with 3.6
It's because we're drawing an outline around the plugin when it has focus. We need to stop that outline from creating scrollbars.
Hmm.  Nothing in that range jumps out at me...
We don't need a regression range. The bug was caused when I added support for 'outline' on plugins.
Yeah, so I guess this bug is invalid then.
So do u know how it would be fixed?
It's a real bug, it just depends on bug 542595.
(In reply to comment #16)
> We don't need a regression range. The bug was caused when I added support for
> 'outline' on plugins.

Though the relevant bit in the regression range seems like http://hg.mozilla.org/mozilla-central/rev/2676947cb41c , which also makes sense.
Summary: Java applet with 100% width and height enables scrollbars → Firefox 3.6 regression: plugins get a focus outline (which causes scrollbars)
Sounds like a layout bug, moving.
Component: Plug-ins → Layout
QA Contact: plugins → layout
Attached patch temporary fix (obsolete) — — Splinter Review 
Assignee: nobody → roc

        Attachment #425147 -
        Flags: review?(dbaron)

    
    

    
  
Comment on attachment 425147 [details] [diff] [review]
temporary fix

You might want to leave them, and instead add:

:not(:-moz-suppressed):not(:-moz-user-disabled):not(:-moz-broken)

to both of those selectors.

But you should double-check with bzbarsky about that.


If that doesn't make sense, this is ok, though.

        Attachment #425147 -
        Flags: review?(dbaron) → review+

    
    

    
  
Hmm.  I'm frankly not quite sure why we have a list of tags here instead of just *:focus....

In any case, I'm fine with the selector proposed in comment 23.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix v2
        — 
          — Splinter Review
      

    


  
I'll go with that selector.

I don't know why "embed" isn't here. Or maybe it should be *|*:focus as you say...

        Attachment #425147 -
        Attachment is obsolete: true

        Attachment #425181 -
        Flags: review+
Whiteboard: [needs landing]

    
    

    
  
Dbaron didn't like a similar patch in (the similar) bug 449915.

    
    

    
  
Yeah. I think the difference here is that this is a temporary fix to work around a known layout issue (bug 542595). When that issue is fixed, we'll revert this patch.

    
    

    
  
I don't think this is a security and stability blocker, but we'd like a regression fix for Firefox 3.6. Since the patch here isn't nominated for approval, I'm unclear if you think we should take this temporary fix on the branch, roc, or wait for the fuller one.
blocking1.9.2: ? → -

          status1.9.2:
          --- → wanted

    
    

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

    
    

    
  
Backed out. The test failed, not sure why yet.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [needs review]
Whiteboard: [needs review]

    
    

    
  
I'm not really sure why you wanted to add :not(:-moz-suppressed):not(:-moz-user-disabled):not(:-moz-broken) instead of removing the rule. That selector matches regular plugin instances. I'll reland without that selector.
Whiteboard: [needs landing]

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/1640dcbf1d4c
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]

    
    

    
  
Comment on attachment 425181 [details] [diff] [review]
fix v2

We should take this temporary fix on the branch. The proper fix will never land on the branch.

        Attachment #425181 -
        Flags: approval1.9.2.2?

    
    

    
  
(In reply to comment #27)
> Yeah. I think the difference here is that this is a temporary fix to work
> around a known layout issue (bug 542595). When that issue is fixed, we'll
> revert this patch.

We might even want to revert it when bug 418521 is fixed, if that happens sooner.

    
    

    
  
Comment on attachment 425181 [details] [diff] [review]
fix v2

a1922=beltzner

        Attachment #425181 -
        Flags: approval1.9.2.2? → approval1.9.2.2+
Whiteboard: [needs 192 landing]

    
    

    
  
(In reply to comment #35)
> I'm not really sure why you wanted to add
> :not(:-moz-suppressed):not(:-moz-user-disabled):not(:-moz-broken) instead of
> removing the rule. That selector matches regular plugin instances. I'll reland
> without that selector.

Er, I guess what I meant was:

applet:focus:-moz-suppressed, applet:focus:-moz-user-disabled, etc.
blocking2.0: ? → final+
Priority: -- → P2

    
    

    
  
(In reply to comment #27)
> Yeah. I think the difference here is that this is a temporary fix to work
> around a known layout issue (bug 542595). When that issue is fixed, we'll
> revert this patch.

It's time to that?

    
    

    
  


  
We should probably change this to "*" or "*|*" after we branch, though.

        Attachment #485426 -
        Flags: review?(roc)

        Attachment #485426 -
        Flags: review?(roc) → review+

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: