1031661 - when newtab is set on preload , zoom level is always on 100%

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Saad Shamsaee]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140626181429

Steps to reproduce:

when i set newtab page zoom level on 90% ...


Actual results:

this setting not saved, and always return to 100%. finally i know that is because of i have set browser.newtab.preload to true.


Expected results:

newtab on preload setting must save.

Comment 1

[Saad Shamsaee]

why not confirm ?

Comment 2

[Mahdi Dibaiee [:mahdi][:mdibaiee]]

I can confirm this, with preload set to true, zoom level resets to 100% but there's no problem without it.

Comment 3

[Saad Shamsaee]

why still not fixed??????
i use always updated aurora, in new updates, tabs browser.newtab.preload is set on true by default, but zoom always resets to 100%

Please fix.

Comment 4

[SUBASH T]

I  tried this bug raised by you the same thing happened to me also.
platform: os-windows-7.
browser: firefox 32(version).

Comment 5

[Ed Lee :Mardak]

This became more of an issue when tiles switched to a fixed size from bug 978338.

Comment 6

[Ed Lee :Mardak]

ttaubert, will bug 1077652 affect this bug as we're changing how preload is implemented?

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Ed Lee :Mardak from comment #6)
> ttaubert, will bug 1077652 affect this bug as we're changing how preload is
> implemented?

I honestly don't know. You can grab one of the try builds if you want and test whether the problem still exists with the new preload mechanism.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Ed Lee :Mardak from comment #6)
> ttaubert, will bug 1077652 affect this bug as we're changing how preload is
> implemented?

Seems like this bug is still around with the new preloader implementation.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1031661-Apply-correct-zoom-level-to-preloaded-co.patch

The problem here is that with tabListener.mStateFlags=0 we will not call XULBrowserWindow.onUpdateCurrentBrowser() when switching to a newly created tab that contains preloaded content.

Creating a tabListener early for the preloading browser seems like too much hassle as it would send progress notifications from the hidden <browser>. Simply setting mStateFlags to "loaded" should be fine and makes us call onUpdateCurrentBrowser(), correctly applying the desired zoom level.

Even if the preloaded browser didn't quite finish preloading yet the now present mTabProgressListener will update mStateFlags soon after with the next onStateChange notification.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

A little too late for 40, but 41 should be doable.

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 8640460 [details] [diff] [review]
0001-Bug-1031661-Apply-correct-zoom-level-to-preloaded-co.patch

>--- a/browser/base/content/tabbrowser.xml
>+++ b/browser/base/content/tabbrowser.xml
>@@ -1845,16 +1845,26 @@
>             var tabListener = this.mTabProgressListener(t, b, uriIsAboutBlank);
>             const filter = Components.classes["@mozilla.org/appshell/component/browser-status-filter;1"]
>                                      .createInstance(Components.interfaces.nsIWebProgress);
>             filter.addProgressListener(tabListener, Components.interfaces.nsIWebProgress.NOTIFY_ALL);
>             b.webProgress.addProgressListener(filter, Components.interfaces.nsIWebProgress.NOTIFY_ALL);
>             this.mTabListeners[position] = tabListener;
>             this.mTabFilters[position] = filter;
> 
>+            // Set mStateFlags manually when using preloaded content as there
>+            // was no progress listener around when the content started loading.
>+            // If the content didn't quite finish loading yet, mStateFlags will
>+            // very soon be overridden with the correct value and end up at
>+            // STATE_STOP anyway again.
>+            if (usingPreloadedContent) {
>+              tabListener.mStateFlags = Ci.nsIWebProgressListener.STATE_STOP |
>+                                        Ci.nsIWebProgressListener.STATE_IS_REQUEST;
>+            }

This looks like it should be part of mTabProgressListener, maybe as opt-in behavior depending on another parameter...

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1031661-Apply-correct-zoom-level-to-preloaded-co.patch, v2

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 8642450 [details] [diff] [review]
0001-Bug-1031661-Apply-correct-zoom-level-to-preloaded-co.patch, v2

>       <!-- A web progress listener object definition for a given tab. -->
>       <method name="mTabProgressListener">
>         <parameter name="aTab"/>
>         <parameter name="aBrowser"/>
>         <parameter name="aStartsBlank"/>
>+        <parameter name="aFakeStateFlags"/>
>         <body>
>         <![CDATA[
>+          let stateFlags = 0;
>+          // Initialize mStateFlags to non-zero e.g. when creating a progress
>+          // listener for preloaded browsers as there was no progress listener
>+          // around when the content started loading. If the content didn't
>+          // quite finish loading yet, mStateFlags will very soon be overridden
>+          // with the correct value and end up at STATE_STOP again.
>+          if (aFakeStateFlags) {
>+            stateFlags = Ci.nsIWebProgressListener.STATE_STOP |
>+                         Ci.nsIWebProgressListener.STATE_IS_REQUEST;
>+          }

You might as well just call aFakeStateFlags aWasPreloadedBrowser or something like that instead, for more clarity? Not sure if there could be other use cases where we'd want to use that parameter.

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Dão Gottwald [:dao] from comment #14)
> You might as well just call aFakeStateFlags aWasPreloadedBrowser or
> something like that instead, for more clarity? Not sure if there could be
> other use cases where we'd want to use that parameter.

Fair enough, will do.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/300ab34e6696

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/300ab34e6696

Comment 19

[Nazir Ahmed Sabbir [:NaSb]]

Successfully reproduce this bug on Firefox-31.0b5 (27-Jun-2014) by following comment 0 in Linux,64 bit

This Bug is now verified as fixed on Latest Firefox beta 42.0b3

Build ID:  20151001142456
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0

Comment 20

[Md. Rahimul Islam]

I have successfully reproduce this bug on firefox nightly 33.0a1 (2014-06-26)
with windows 10 (32 bit)
Mozilla/5.0 (Windows NT 6.3; rv:33.0) Gecko/20100101 Firefox/33.0

I found this fix on latest firefox realse 42.0

Mozilla/5.0 (Windows NT 10.0; rv:42.0) Gecko/20100101 Firefox/42.0

Build ID : 20151029151421

[bugday:20151118]

Comment 21

[Mohammad Maruf Islam]

As this bug's fix is verified on both Linux (Comment 19) and Windows (Comment 20), I am marking this bug as verified.