470441 - Can't switch tabs between a website and about:config

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[cmtalbert]

= Steps =
1. Open a tab with about:config
2. Go into the preferences area, search for jit (probably anything will do)
3. Open a new tab to a website (any website)
4. Switch back to about:config

= Actual =
The URL bar updates to say about:config, but the content area still displays the previous website

= Expected =
You would switch back to about:config

= Notes =
You don't have this problem with about: just about:config.  I also don't have the problem with about:mozilla or about:buildconfig.  It seems to be specific to about:config.

Version: Mozilla/5.0 (X11; U; Linux armv6l; en-US;rv:1.9.2a1pre) Gecko/20081219 Fennec/1.0a2pre

Comment 1

[:Gavin Sharp [email: [e-mail]]]

This was probably fixed by bug 470477 (I hadn't yet seen this bug when I filed that one). Clint, can you retest?

Comment 2

[Ben Combee (:bcombee)]

I still see this in a Win32 Fennec build I made this morning.

Comment 3

[Al Billings [:abillings - ex-MoCo]]

I'm still seeing this months later...

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Path to force the redraw

Patch based on the one from bug 470477 but forcing a redraw a the end of selectedTab seems to work (otherwise we are in a updateBatch that prevent redrawing (as I have understand) and sometimes there is no MozAfterPaint after the selectTab call who init a refresh)

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

yes, commenting out the ws.beginUpdateBatch / ws.endUpdateBatch allows the tab redraw to happen during a switch too.

I worry that your approach might be adding more painting for non-XUL content situations (switching between to normal webpages)

We should look into why the updateBatch is not handling this correctly and fix that.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #5)
> yes, commenting out the ws.beginUpdateBatch / ws.endUpdateBatch allows the tab
> redraw to happen during a switch too.
> 
Yes. It seems that  updateViewportSize (that do the redraw) end before the redraw on WidgetStack#861 cause of the 'this._skipViewportUpdates' set by beginUpdateBatch. But if you only disable the ws.[begin|end]UpdateBatch you can see a part of the display from the previous tab.

> I worry that your approach might be adding more painting for non-XUL content
> situations (switching between to normal webpages)
> 
I've to admit that :)

In fact it's not a complete patch, I currently investigating this one and https://bugzilla.mozilla.org/show_bug.cgi?id=492084 that i can reproduce easily (even with my ugly patch)

> We should look into why the updateBatch is not handling this correctly and fix
> that.

I suspect something is wrong in WidgetStack, in fact the previously mentioned patch and this one are probably related (in both cases if I use my mouse wheels the content finally appears)

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Update patch

This one work better. It seems to be a regression since bug 470477.

In fact the actual Fennec behavior doesn't render the current page if i go on two web page that have the same size :
for example :
 - Open a new tab and go to google.com
 - Open a second tab and go to google.fr
 - switch tab. Content hasn't changed

What I've understand so far is [begin|end]UpdateBatch block the redrawing mechanism and when endUpdateBatch is called he look if there is a diff in viewport since the UpdateBatch (patch from bug 470477 want to fake the size to force a redraw) to redraw the entire pageBounds. 

This patch add a ws.ensureRedrawAfterUpdateBatch property

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 379163 [details] [diff] [review]
Update patch

I like the idea of allowing the code to force a redraw. I'd like it more if we added an optional boolean to ws.endUpdateBatch. Make the boolean default to | false | if nothing is passed in.

ws.endUpdateBatch(forceRedraw)

then pass | true | in the tab switching code.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Updated patch

Add a boolean parameter to endUpdateBatch
Save the value for nested calls

Comment 10

[Ben Combee (:bcombee)]

Isn't this logic in your patch

    if (!this._redrawAfterBatch && aForceRedraw)
      this._redrawAfterBatch = aForceRedraw;

the same as the simpler form

    this._redrawAfterBatch = this._redrawAfterBatch || aForceRedraw;

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Updated patch

Right :)

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 380153 [details] [diff] [review]
Updated patch

I going to rename "_redrawAfterBatch" to "_forceViewportUpdate" when I land it.

Thanks Vivien!

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/mobile-browser/rev/f70b68090b41

Comment 14

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.2b1pre) Gecko/20090928
Fennec/1.0b4pre

and

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20090928
Fennec/1.0b4pre