Closed Bug 97580 Opened 23 years ago Closed 19 years ago

joystick.fr always reload on tab switch in gtknotebook

Categories

(Core Graveyard :: Embedding: GTK Widget, defect)

Product:
Core Graveyard
Component:
Embedding: GTK Widget
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: christophe.fergeau, Assigned: blizzard)

Details

(Keywords: fixed1.8)

Attachments

(4 files)

Testcase as attachment
207 bytes, text/html
Details
Patch to bypass Resize
1.51 KB, patch
blizzard
: review-
Details | Diff | Splinter Review
Avoid call to Resize if dimensions didn't change
2.40 KB, patch
blizzard
: review+
bryner
: superreview+
Details | Diff | Splinter Review
revert attachment 155293
3.46 KB, patch
blizzard
: review+
roc
: superreview+
cbeard
: approval1.8b4+
Details | Diff | Splinter Review 
With galeon, if you go to www.joystick.fr, wait until the page is fully loaded,
and then switch tab, the pages reload. gtktestembednotebook also exhibit this
behaviour (I modified it to display www.joystick.fr instead of www.mozilla.org),
I waited for this page to be loaded, and then switch tabs, and it reloaded
Marking these all WORKSFORME sorry about lack of response but were very
overloaded here. Only reopen the bug if you can reproduce with the following steps:

1) Download the latest nightly (or 0.9.6 which should be out RSN)
2) Create a new profile
3) test the bug again

If it still occurs go ahead and reopen the bug. Again sorry about no response
were quite overloaded here and understaffed.
Status: UNCONFIRMED → RESOLVED
Closed: 23 years ago
Resolution: --- → WORKSFORME
Accidentally marked, reopening.
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
I dont think this is our probem its Galeon's tabs that are malfunctioning....and
i cant seem to repeat it anyway..Marking invalid.
Status: UNCONFIRMED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → WORKSFORME
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
The problem is also reproducible with the TestGtkEmbedNotebook in
mozilla/embedding/browser/gtk/tests 
Step to reproduce : 

Run ./TestGtkEmbedNotebook  http://www.joystick.fr/news/fiche_news.php3?ID=3960
Switch to the tab displaying the site
Wait until it finishes loading
Switch to the 'gtk_label' tab
Come back to the 'mozembed' tab. 
The site reloads.
The site has:

onresize="if (navigator.appName == 'Netscape'){location.reload()};init();" 

Please check whether switching tabs in galeon fires the onresize event on the
page in the tab...
Since mozilla (the browser) doesn't suffer this and galeon does I suggest that
you file a bug in http://bugzilla.gnome.org/ and see if they refer you back here
(with more information). Just for the record, yes galeon (1.2) does fire a
onresize event when tabs are switched:

<html>
<head><title>Onresize</title></head>
<body onresize="if (navigator.appName == 'Netscape') {document.write('Resize!')}">
</body>
</html>

Resize was printed when the tab was switched and some strangeness appeared in
the location bar too...
Sitsofe, your test does not prove anything.
It could be the gtkmozembed code firing that resize event.

I looked at TestGtkEmbedNotebook and it doesn't have any special handling for
onresize or any event. Everything is done entirely by the gtk mozilla widget.
As I said in comment #4, the problem in www.joystick.fr (which will soon close
:( also happens in TestGtkNotebook from Mozilla.
In my initial report, I talked about galeon, but imo it's definitely a mozilla
bug since it's reproduceable in TestGtkNotebook. It probably fires onresize
events on tab switches
confirming so blizzard will actually look at this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file Testcase as attachment — 

    
    

    
  


  
Patch to bypass Resize when we get a gtk_widget_size_allocate message for the
current size.

        Attachment #129208 -
        Flags: review?(blizzard)

    
    

    
  
Chris, any chance of review on this?  Should I try to look for someone else? 
This patch has been rotting for over 6 months!

    
    

    
  
Comment on attachment 129208 [details] [diff] [review]
Patch to bypass Resize

>Index: embedding/browser/gtk/src/gtkmozembed2.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed2.cpp,v
>retrieving revision 1.29
>diff -u -d -b -w -U16 -r1.29 gtkmozembed2.cpp
>--- embedding/browser/gtk/src/gtkmozembed2.cpp	22 May 2003 04:00:56 -0000	1.29
>+++ embedding/browser/gtk/src/gtkmozembed2.cpp	5 Aug 2003 04:12:06 -0000
>@@ -646,36 +646,43 @@
> static void
> gtk_moz_embed_size_allocate(GtkWidget *widget, GtkAllocation *allocation)
> {
>   GtkMozEmbed  *embed;
>   EmbedPrivate *embedPrivate;
>   
>   g_return_if_fail(widget != NULL);
>   g_return_if_fail(GTK_IS_MOZ_EMBED(widget));
> 
>   embed = GTK_MOZ_EMBED(widget);
>   embedPrivate = (EmbedPrivate *)embed->data;
>   
>   widget->allocation = *allocation;
> 
>   if (GTK_WIDGET_REALIZED(widget))
>   {
>+    PRInt32 aX, aY;

aX and aY are argument names, and not the right names anyway.  Should just be
width, height.

>+    embedPrivate->mWindow->mBaseWindow->GetSize(&aX, &aY);
>+
>     gdk_window_move_resize(widget->window,
> 			   allocation->x, allocation->y,
> 			   allocation->width, allocation->height);
>+
>+    if((allocation->width != (gint) aX) || (allocation->height != (gint) aY))
>+    {
>     embedPrivate->Resize(allocation->width, allocation->height);
>+    }
>   }
> }

Careful with tabs here.  The indentation is not correct.

        Attachment #129208 -
        Flags: review?(blizzard) → review-

    
    

    
  


  
Add EmbedPrivate::GetSize to match Resize and don't call Resize if the
allocation didn't change dimensions.

    
  

        Attachment #155293 -
        Flags: review?(blizzard)

    
  

        Attachment #155293 -
        Flags: review?(blizzard) → review+

    
  

        Attachment #155293 -
        Flags: superreview?(bryner)

    
    

    
  
Interestingly this doesn't happen for me anymore using mozilla trunk, I think it
was fixed somewhere between 20041012 and 20041020, possibly bug 227361, I have
discounted bug 258917

        Attachment #155293 -
        Flags: superreview?(bryner) → superreview+

    
    

    
  
Checked in
Status: NEW → RESOLVED
Closed: 23 years ago → 20 years ago
Resolution: --- → FIXED

    
    

    
  
This patch should be backed out, since it breaks flash.
To see this, browser with TestGtkEmbed to http://www.macromedia.com and observe
the grey area where the flash movies should be. Resize the window, and see the
flash appear.
The patch is also not necessary anymore to fix the original problem (reload loop
from onresize), since that was fixed in another bug and doesn't occur anymore
even without this patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  


  
Back out the checkin, since it breaks flash (and other plugins); see comment
17.

        Attachment #180941 -
        Flags: superreview?(bryner)

        Attachment #180941 -
        Flags: review?(mpgritti)

    
    

    
  
Comment on attachment 180941 [details] [diff] [review]
revert attachment 155293 [details] [diff] [review]

Changing review request to blizzard, since he reviewed the original patch...

        Attachment #180941 -
        Flags: review?(mpgritti) → review?(blizzard)

    
  

        Attachment #180941 -
        Flags: review?(blizzard) → review+

    
    

    
  
Comment on attachment 180941 [details] [diff] [review]
revert attachment 155293 [details] [diff] [review]

Trying another sr. bryner seems to be busy, and we really need to get this into
1.8. It's a hugely visible regression.

        Attachment #180941 -
        Flags: superreview?(bryner) → superreview?(roc)

        Attachment #180941 -
        Flags: superreview?(roc) → superreview+

    
    

    
  
Comment on attachment 180941 [details] [diff] [review]
revert attachment 155293 [details] [diff] [review]

Embed-only. Fixes a highly visible regression (flash and other plugins
initially showing up blank until you resize the window); only reverts code to
previous state so no risk.

        Attachment #180941 -
        Flags: approval1.8b4?

    
  

        Attachment #180941 -
        Flags: approval1.8b4? → approval1.8b4+

    
    

    
  
HEAD:
Checking in embedding/browser/gtk/src/EmbedPrivate.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.cpp,v  <--  EmbedPrivate.cpp
new revision: 1.54; previous revision: 1.53
done
Checking in embedding/browser/gtk/src/EmbedPrivate.h;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.h,v  <--  EmbedPrivate.h
new revision: 1.26; previous revision: 1.25
done
Checking in embedding/browser/gtk/src/gtkmozembed2.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed2.cpp,v  <--  gtkmozembed2.cpp
new revision: 1.44; previous revision: 1.43
done

MOZILLA_1_8_BRANCH:
Checking in embedding/browser/gtk/src/EmbedPrivate.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.cpp,v  <--  EmbedPrivate.cpp
new revision: 1.53.2.1; previous revision: 1.53
done
Checking in embedding/browser/gtk/src/EmbedPrivate.h;
/cvsroot/mozilla/embedding/browser/gtk/src/EmbedPrivate.h,v  <--  EmbedPrivate.h
new revision: 1.25.12.1; previous revision: 1.25
done
Checking in embedding/browser/gtk/src/gtkmozembed2.cpp;
/cvsroot/mozilla/embedding/browser/gtk/src/gtkmozembed2.cpp,v  <--  gtkmozembed2.cpp
new revision: 1.43.8.1; previous revision: 1.43
done


    
  
Keywords: fixed1.8

    
    

    
  
(In reply to comment #10)
> Created an attachment (id=129205) [edit]
> Testcase as attachment
> 

This seems fixed on trunk and 1.8 branch; switching tabs in Epiphany no longer
causes spurious resize events.

    
    

    
  
marking fixed, then.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 19 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: