Closed Bug 331684 Opened 18 years ago Closed 18 years ago

applet does not shrink with page when in table and width set to 100% by code.

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: adam, Assigned: dbaron)

References

(
URL
)

Details

(Keywords: fixed1.8.1, regression, testcase, Whiteboard: [reflow-refactor][would take patch][patch])

Attachments

(3 files)

testcase
1.78 KB, text/html
Details
trivial applet which just paints a red cross.
598 bytes, application/x-java-applet
Details
patch
1.34 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
beltzner
: approval1.8.1+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060324 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060324 Firefox/1.6a1

An <applet> which has it's width set to 100% will not shrink below the width chosen when it's size was first set to 100%.
This bug happens when the applet is inside a table cell.
This bug happens when the applet's width is changed in javascript to 100%.


Reproducible: Always

Steps to Reproduce:
1.Maximize browser window.
2.Go to http://www.jigzone.com/xmockup/bugs/resizeTestFF.php
3.Reduce the size of the browser window.
4.Notice that a horizontal scroll bar has appeared.

Actual Results:  
Applet will not shrink below the width it was first displayed despite having a style width of 100%

Expected Results:  
A horizontal scroll bar should not appear, applet should shrink right down. (see example page under IE or Opera or Firefox 1.0)

With a simple example of an <applet style="width:100%"> directly on the <body> everything works as expected.

Things go wrong when the applet is inside a table cell and the applet width is set dynamically after the page has loaded.

On build 20060324 there is a misalignment problem loading the example page which is fixed by clicking the browser back the forwards buttons.
Component: General → Layout
Product: Firefox → Core
Version: unspecified → 1.7 Branch
QA Contact: general → layout
This is an old regression. I can't see the bug in Mozilla1.7.12, but I can see the bug in a 2004-10-10 build. A smaller regression range could be useful here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, testcase
Version: 1.7 Branch → Trunk
Attached file testcase — 
Page needs attached java applet. update <applet classpath= to point to .java file path

    
    

    
  
So for what it's worth, I do see the bug in a 1.8 branch build, but not with a current trunk build.  Is this still an issue?

    
    

    
  
Ria, yes, that's how it's supposed to look like.

I can see this issue in current trunk build:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060714 Minefield/3.0a1

    
    

    
  
In that case it is a regression between 30-May-2004:09 and 2-Jun-2004:09.

    
    

    
  
Hmm.  On Linux, I definitely see this with branch but not trunk.

Do any of you guys have reflow branch builds on hand?  Does that help?

    
    

    
  
(In reply to comment #8)
> Do any of you guys have reflow branch builds on hand?  Does that help?

I have a reflow branch build, but mingw doesn't support building with Java, so that doesn't help :(


    
    

    
  
The most likley candidate for introducing this regression for the dates 30-May-2004:09 and 2-Jun-2004:09 given by Ria is <a href=="30-May-2004:09 and 2-Jun-2004:09">217715</a> but I can't see how it is related.

After a long learning curve and studying reflow debug output, I can 'fix' the bug by changing the nsObjectFrame::GetDesiredSize code FROM:
if (aMetrics.mComputeMEW){
    aMetrics.mMaxElementWidth = aMetrics.width;
}

TO:
if (aMetrics.mComputeMEW){
    if(eStyleUnit_Percent == (*(aReflowState.mStylePosition)).mWidth.GetUnit())
        aMetrics.mMaxElementWidth = 0;
    else
        aMetrics.mMaxElementWidth = aMetrics.width;
}

This makes an <APPLET> reflow similar to that of an <IMG> or <INPUT> when width is % and it's inside a cell.

The change does not seem to break anything so far as I have tested, but perhaps I'm missing something?



    
    

    
  
Yeah, that might be exactly what we want to do.  All that code is gone on the reflow branch, though; you'd want a similar hack in GetMinWidth().

But that raises the question of whether we want that sort of thing in general; at the moment some replaced elements do it and others do not; what does IE do?

Note that on reflow branch I did reproduce this behavior for the form controls that used to have it, but the behavior for images has changed.  Not sure which is correct.
Whiteboard: [reflow-refactor]

    
    

    
  
All 'modern' browsers support dynamic resizing of applets with % widths, and even Firefox 1.0 did it. (some deficient browsers don't support the resizing of applets at all, but thats a LiveConnect issue)

I don't know why it worked on Firefox 1.0 and not now. The code I modified has not changed since the begining of CVS time.

    
    

    
  
On the question of whether we want that sort of thing in general, I ran an updated testcase on a variety of browsers which reveals that Firefox 2 is the odd one out. http://www.jigzone.com/xmockup/bugs/resizeTestFF.php

Firefox 1.0.6; Internet Explorer 7.0, 6.0, 5.5, 5.01; Netscape 8.02, 7.0, 7.1; Safari 2.0.3; Opera 8.01; and Konqueror 3.4.2 are consistent on this.

Can this be fixed for firefox 2.0? then at least this real-world webpage would not wrongly develop a horizontal scroll bar: http://www.jigzone.com/z.php

To make a patch less 'dangerous' the code could be:
if(   atom == nsHTMLAtoms::applet 
   && eStyleUnit_Percent == (*(aReflowState.mStylePosition)).mWidth.GetUnit())
     	aMetrics.mMaxElementWidth = 0;

This would limit the change to java applets with % widths which are few and far between on the internet.   


    
    

    
  
> On the question of whether we want that sort of thing in general

"In general" means "for all replaced elements, not just <applet>".

As far as fixing this for Gecko 1.8.1, that's up to the branch drivers.  The patch described in comment 13 does look correct, so the question is whether we want to make this behavior change on branch (where we generally promise Gecko compat with Gecko 1.8).  Frankly, I think we should consider making this change, unless dbaron objects.
Flags: blocking1.8.1?

    
    

    
  
(In reply to comment #10)
> TO:
> if (aMetrics.mComputeMEW){
>     if(eStyleUnit_Percent == (*(aReflowState.mStylePosition)).mWidth.GetUnit())
>         aMetrics.mMaxElementWidth = 0;
>     else
>         aMetrics.mMaxElementWidth = aMetrics.width;
> }

Note that nsHTMLReflowMetrics::SetMEWToActualWidth does this.

    
    

    
  
Not going to block on this as it isn't a regression from 1.5, but we'd definitely be interested in taking the fix if someone wants to whip up the patch.
Flags: blocking1.8.1? → blocking1.8.1-
Whiteboard: [reflow-refactor] → [reflow-refactor][would take patch]
Assignee: nobody → dbaron

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  
This patch exactly matches the way we fix the same problem in a bunch of other places.  Thanks for pointing out what the problem was.

I haven't tested this yet.

    
    

    
  
Comment on attachment 233844 [details] [diff] [review]
patch

I've tested that this fixes the problem on http://www.jigzone.com/xmockup/bugs/resizeTestFF.php

        Attachment #233844 -
        Flags: superreview?(bzbarsky)

        Attachment #233844 -
        Flags: review?(bzbarsky)
Whiteboard: [reflow-refactor][would take patch] → [reflow-refactor][would take patch][patch]

    
    

    
  
Comment on attachment 233844 [details] [diff] [review]
patch

Looks good!

        Attachment #233844 -
        Flags: superreview?(bzbarsky)

        Attachment #233844 -
        Flags: superreview+

        Attachment #233844 -
        Flags: review?(bzbarsky)

        Attachment #233844 -
        Flags: review+

    
    

    
  
Checked in to trunk.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED

    
    

    
  
Comment on attachment 233844 [details] [diff] [review]
patch

a=beltzner on behalf of drivers for the mozilla_1_8_branch

        Attachment #233844 -
        Flags: approval1.8.1? → approval1.8.1+

    
    

    
  
Checked in to MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: