1319366 - Clicking a link while presenting Google slides makes the navigation bar go black

Status: RESOLVED DUPLICATE of bug 1341276

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Guilherme Gonçalves [:ggp]]

Attachment: bug.png

Reproduced on a freshly-installed Nightly build, on a Nexus 5X phone running Android 7.0

Steps to reproduce:
1- Open any Google presentation that contains a link (example: https://docs.google.com/presentation/d/145gnd-wT8kbmA9YuXaCWYB9LYaKEFM7WmIkKHzKgP6Y/edit#slide=id.p)
2- Go into fullscreen/presentation mode
3- Click a link in the presentation

A new tab gets opened as expected, but its navigation bar is black and unusable (see attached screenshot). I can only recover from this by manually restarting the app.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Yep, I see this too on a Pixel XL using Nightly.

Comment 2

[Nevin Chen(Not active on Bugzilla)]

Hi Jim
I found that when the link is clicked, a new tab is opened cause I can receive the 
"type":"Tab:Added" event, but the app stays in full screen mode. IMO it make more sense for  GeckoView to leave full screen mode when opening a new link. How do you think?

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Makes sense. More specifically, we should leave full screen mode when opening new tab / switching tabs.

Comment 4

[Nevin Chen(Not active on Bugzilla)]

Hi Ray
Would you please help advise how can I change state if the full screen icon in videocontrols.xml?

I want to make the app leave full screen mode when a new tab is added.
I can do it in the Java world but I also want to change the status of the control wedget in JS

Comment 5

[Ray Lin[:ralin]]

Hi Nevin

I'm afraid that the controls doesn't belong to video controls(videocontrols.xml), video control shows up only when <video>/<audio> tags embedded in content. Could you provide me more details about your intention? Thanks.

Comment 6

[Nevin Chen(Not active on Bugzilla)]

I've found this might be helpful for Google slide widghet. I'll test it tomorrow.
https://developer.mozilla.org/zh-TW/docs/Web/API/Document/exitFullscreen

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Hi Jim
I can simple show the Chrome UI in Java side using 
                if (ActivityUtils.isFullScreen(this)){

                    LayerView layerView = mLayerView;
                    if (layerView != null) {
                        layerView.setFullScreenState(FullScreenState.NONE);
                    }
                    setFullScreen(false);
                }
in
https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#1814

But I'm not sure if it's better to do this in platform side?
Since platform will know when to leave full screen mode before adding a new tab.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Jim Chen [:jchen] [:darchons] from comment #3)
> Makes sense. More specifically, we should leave full screen mode when
> opening new tab / switching tabs.

We should already have code for this in browser.js.

This bug sounds a bit like it could be related to bug 1222798 - So I'll flag esawin additionally. :)

Comment 9

[Eugen Sawin [:esawin]]

It's not directly related to bug 1222798, I couldn't find any code path for exiting fullscreen mode when adding a new tab.

I think the solution presented in comment 7 would be acceptable.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Eugen Sawin [:esawin] from comment #9)
> It's not directly related to bug 1222798, I couldn't find any code path for
> exiting fullscreen mode when adding a new tab.

If we open a new tab and do not switch to it (that's where the code is, right?) then I guess we do not need to leave fullscreen mode? Only when actually switching to a (new) tab?

Comment 11

[Eugen Sawin [:esawin]]

(In reply to Sebastian Kaspari (:sebastian) from comment #10)
> (In reply to Eugen Sawin [:esawin] from comment #9)
> > It's not directly related to bug 1222798, I couldn't find any code path for
> > exiting fullscreen mode when adding a new tab.
> 
> If we open a new tab and do not switch to it (that's where the code is,
> right?) then I guess we do not need to leave fullscreen mode? Only when
> actually switching to a (new) tab?

That's right, although I'm not sure we can easily support this use case with the current mechanics.
In this particular case (Google Docs presentation), opening a tab in background is not an option.

Comment 12

[Nevin Chen(Not active on Bugzilla)]

The fix in comment 7 got some problem. Only changing the Android UI not really make the browser leave full screen mode and will cause some strange behavior.
I end up handing it in JS side
                if (ActivityUtils.isFullScreen(this)) {
                    GeckoAppShell.notifyObservers("FullScreen:Exit", null);
                }
and in https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1786
change 
          browser.contentDocument.exitFullscreen();      
to
          document.exitFullscreen();      

But this still got some problem.
The new added tab will still show the content the last loaded tab. I'll have to switch to another tab and switch back to make it display. 
I can't find the cause of this behavior since all JS events seems to be triggered correctly (tab:added->tab:selected .... even thumbnail is created correctly).

Would you please give me some hint? I'll try it next week. Thank you!

Comment 13

[Eugen Sawin [:esawin]]

(In reply to Nevin Chen [:nechen] from comment #12)
> The new added tab will still show the content the last loaded tab. I'll have
> to switch to another tab and switch back to make it display. 
> I can't find the cause of this behavior since all JS events seems to be
> triggered correctly (tab:added->tab:selected .... even thumbnail is created
> correctly).

Is it possible that you enter BrowserApp.selectTab while in fullscreen (and therefore set fullscreenTransitionTab) and then never receive a "fullscreenchange" event?

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Looks like you are getting help from Eugen and Sebastian.

Comment 15

[Nevin Chen(Not active on Bugzilla)]

Attachment: index.html

If you go to the sample page that Julian has built for me https://nevin-lab.appspot.com/
You'll find that when clicking on the link and open a new tab, everything is working fine (using nightly)

Comment 16

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

Review commit: https://reviewboard.mozilla.org/r/104506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104506/

Comment 17

[Nevin Chen(Not active on Bugzilla)]

Hi Eugen.
Please correct me if I'm wrong....
I can't find when BrowserApp.selectTab is called while adding a new tab... so it might not be the cause?

Hi Jim.
I still need your wisdom here. 
When fullscreen,
If without target="_blank", clicking a link in full screen will correctly exit full screen and open the url in the same tab. 
But if I have two tabs A,B opened and loaded. When tab A is fullscreen and I click a link with target="_blank", a new tab C will open and layer view will show the content of B....
Please find my patch and give me some hint :) ... thanks!

Comment 18

[Nevin Chen(Not active on Bugzilla)]

Attachment: logcat

btw, please find the event sequence in the attachment. The event are the same for with/without fullscreen when target="_blank" The log is from here
https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/Tabs.java#525

I've also called "Tabs.getInstance().getSelectedTab().getId()" in 
https://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#1827
The tab is is also the latest. But layer view still shows the wrong content.

Please give me some advice. Thank you!

Comment 19

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

From the comments it looks like this is not the final patch yet? Just reflag me if needed. :)

Comment 20

[Nevin Chen(Not active on Bugzilla)]

Hi Jim
I just found that if I put a break point here in WebIDE
https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#3337
Just pause and continue to run...my patch will magically work....Could it related to Gecko rendering issue? 
Cause I can't sense any where else could go wrong...
Thanks!

Comment 21

[Nevin Chen(Not active on Bugzilla)]

(In reply to Nevin Chen [:nechen] from comment #20)

> I just found that if I put a break point here in WebIDE
> https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/
> browser.js#3337

Also I found If I put a break point here in WebIDE , the app will show correct content:
http://searchfox.org/mozilla-central/source/mobile/android/chrome/content/browser.js#4192
Both w/o the break point/pause will show me the same message, 
But I can't find where this onLocationChange gets called so I can't debug further...

Comment 22

[Eugen Sawin [:esawin]]

(In reply to Nevin Chen [:nechen] from comment #17)
> Please correct me if I'm wrong....
> I can't find when BrowserApp.selectTab is called while adding a new tab...
> so it might not be the cause?

You're right, it's not called by default, I just tried to picture a scenario that would describe what you see, assuming changed behavior with your patch.

> But I can't find where this onLocationChange gets called so I can't debug
> further...

browser.js registers as a listener for webprogress events and the webprogress manager calls "onLocationChange", "onStatusChange", etc., on the registered listeners.

Comment 23

[(inactive) Jim Chen [:jchen] [:darchons]]

Sounds like a possible race between the UI thread and the Gecko thread. Have you tried your patch with the latest mozilla-central? Bug 1329268 just landed that tried to address some of the possible race conditions.

Comment 24

[Nevin Chen(Not active on Bugzilla)]

I've pull the latest code. But it still didn't work. 

I've check the Desktop code from :xidorn. He pointed out the desktop code register tab's open/close/select event and leave full screen mode when event happens.[1]

I think my patch is similar.
I even try to leave fullscreen mode here [2] using 
BrowserApp.deck.addEventListener("TabOpen", (e) => { document.exitFullscreen();});

or add below code to [3]... but it still shows the last tab content.
case "TabOpen":  
    document.exitFullscreen();
    break;
I think there's something wrong between [4] or [5] or [6]
Do you have time to look at this? Or Can you give me some advice ?
Thanks again!

[1] https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-fullScreenAndPointerLock.js#445-447
[2] https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#412
[3] https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#6875
[4] https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsNativeAppSupportWin.cpp#1209
[5] https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsNativeAppSupportWin.cpp#1433
[6] https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsNativeAppSupportWin.cpp#1463

Comment 25

[(inactive) Jim Chen [:jchen] [:darchons]]

I'm not sure I understand entirely what's going on from the comments. Can you summarize the bug? So you start with a fullscreen page, then click a link, and then what happens?

Comment 26

[Nevin Chen(Not active on Bugzilla)]

Hi Jim
Sorry I should use permalink since the code changes everyday.

STR
1. Go to Google slide [1] and open a link.
2. The app will show correct content but didn't leave full screen mode.

Fix:
I tried to fix it by adding a fullscreen check whenever a new tab is created. So I add below lines in [2]

    let openerWindow = (aFlags & Ci.nsIBrowserDOMWindow.OPEN_NO_OPENER) ? null : aOpener;
    // =====add below lines========
    if (documet.fullscreenElement){
        document.exitFullscreen();
    }
    // =====add above lines========
    let tab = BrowserApp.addTab(aURI ? aURI.spec : "about:blank", { flags: loadflags,
                                                                      referrerURI: referrer,
                                                                      external: isExternal,
                                                                      parentId: parentId,
                                                                      opener: openerWindow,
                                                                      selected: true,
                                                                      isPrivate: isPrivate,
                                                                      pinned: pinned });
Fix Result:
App will leave fullscreen mode and display the navigation bar correctly, but it will show the last opened tab( e.g. tab A, B, C is opened. Click a link on tab A while A is in full screen mode, a new tab D will open, but the app shows the content of tab C)
I think my fix is correct because when I add a breakpoint and pause before I return newURI [3], the app will show correct content (tab D).
I suspect that the error happens before [4] cause if openURI returns too early(without breakpoint in [3]), the app will show wrong content. But I'm totally guessing cause I don't know any of the cpp code. So I need your wisdom to help me point out what could I did wrong or where can I know more about the cpp code(or I even look into the wrong file?)


[1] https://docs.google.com/presentation/d/145gnd-wT8kbmA9YuXaCWYB9LYaKEFM7WmIkKHzKgP6Y/edit#slide=id.p
[2] https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/mobile/android/chrome/content/browser.js#3379
[3] https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/mobile/android/chrome/content/browser.js#3381
[4] https://dxr.mozilla.org/mozilla-central/rev/3cedab21a7e65e6a1c4c2294ecfb5502575a46e3/toolkit/xre/nsNativeAppSupportWin.cpp#1463

Comment 27

[(inactive) Jim Chen [:jchen] [:darchons]]

What happens if you call `document.exitFullscreen` after `BrowserApp.addTab` in [2]? Or add a delay? e.g. `setTimeout(() => document.exitFullscreen(), 0);`

Comment 28

[Nevin Chen(Not active on Bugzilla)]

If I add setTimeout(() => document.exitFullscreen(), 0);  It'll still show the last open tab.
but this time , adding the breaking point won't help.

Comment 29

[(inactive) Jim Chen [:jchen] [:darchons]]

Did you try changing the timeout? Like 100ms or 1s?

Comment 30

[Nevin Chen(Not active on Bugzilla)]

Yes. The result is the same even with breakponts.
Below is the test result
  openURI: function browser_openURI(aURI, aOpener, aWhere, aFlags) {  
    let browser = this._getBrowser(aURI, aOpener, aWhere, aFlags);   // [1]
    setTimeout(() => {                                               // [2][4]              
        document.exitFullscreen()                                    // [7]
    }, 5000);                                                        // [3]    
    return browser ? browser.contentWindow : null;                   // [5]
  },                                                                 // [6]               
[1] Showing a link and the test page in fullscreen mode (http://nevin-lab.appspot.com)
[2]~[6] Showing a blank layer view and black bar above it.
[7] navigation bar( Showing the content of previous opened page. (ToolbarDisplayLayout and status bar is back to normal)

Comment 31

[Nevin Chen(Not active on Bugzilla)]

Yes. The result is the same even with breakponts.
Below is the test result
  openURI: function browser_openURI(aURI, aOpener, aWhere, aFlags) {  
    let browser = this._getBrowser(aURI, aOpener, aWhere, aFlags);   // [1]
    setTimeout(() => {                                               // [2][4]              
        document.exitFullscreen()                                    // [7]
    }, 5000);                                                        // [3]    
    return browser ? browser.contentWindow : null;                   // [5]
  },                                                                 // [6]               
[1] Showing a link and the test page in fullscreen mode (http://nevin-lab.appspot.com)
[2]~[6] Showing a blank layer view and black bar above it.
[7] Showing the content of previous opened page. (ToolbarDisplayLayout and status bar is back to normal)

Comment 32

[Nevin Chen(Not active on Bugzilla)]

I guess setTimeout is not helping here because it runs the anonymous function asynchrously. In above example, openURI returns before document.exitFullscreen() runs so it doesn't work. But if I call document.exitFullscreen() first and pause the breakpoint before openURI returns, it will work.

Comment 33

[(inactive) Jim Chen [:jchen] [:darchons]]

Actually BrowserApp.selectTab already has logic [1] for handling switching to another tab after exiting fullscreen. You should reuse that logic, and delay selecting the tab [2] when adding a new tab.

[1] http://searchfox.org/mozilla-central/rev/d20e4431d0cf40311afa797868bc5c58c54790a2/mobile/android/chrome/content/browser.js#1308
[2] http://searchfox.org/mozilla-central/rev/d20e4431d0cf40311afa797868bc5c58c54790a2/mobile/android/chrome/content/browser.js#1218

Comment 34

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104506/diff/1-2/

Comment 35

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

https://reviewboard.mozilla.org/r/104506/#review110542

Not sure about the timeout. If it's really needed then reflag me.

I'll also add jchen as reviewer. He's been working with you on this patch, so he might have an opinion too. :)

::: mobile/android/chrome/content/browser.js:1217
(Diff revision 2)
> -    if (selected)
> +    if (selected) {
> +        setTimeout(function(e) {
> -      this.selectedTab = newTab;
> +            this.selectedTab = newTab;
> +        }, 500);
> +    }

Do we really need this timeout here too? It looks like the fullscreen transition code will do the selection?

Comment 36

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

https://reviewboard.mozilla.org/r/104506/#review110678

::: mobile/android/chrome/content/browser.js:1218
(Diff revision 2)
>        this._tabs.push(newTab);
>      }
>  
>      let selected = "selected" in aParams ? aParams.selected : true;
> -    if (selected)
> +    if (selected) {
> +        setTimeout(function(e) {

Yeah you don't need this timeout. I think the better fix is to set `aParams.selected` to `false` if we're in fullscreen mode, exit fullscreen, and then call `BrowserApp.selectTab` after we exit fullscreen mode.

::: mobile/android/chrome/content/browser.js:1233
(Diff revision 2)
>  
>      let evt = document.createEvent("UIEvents");
>      evt.initUIEvent("TabOpen", true, false, window, null);
>      newTab.browser.dispatchEvent(evt);
>  
> +    if (document.fullscreenElement) {

Use `selectedBrowser.contentDocument` instead of `document`.

Comment 37

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104506/diff/2-3/

Comment 38

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104506/diff/3-4/

Comment 39

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

https://reviewboard.mozilla.org/r/104506/#review111618

::: mobile/android/chrome/content/browser.js:434
(Diff revision 4)
>  
>        if (this.fullscreenTransitionTab) {
>          // Tab selection has changed during a fullscreen transition, handle it now.
>          let tab = this.fullscreenTransitionTab;
>          this.fullscreenTransitionTab = null;
>          this._handleTabSelected(tab);

Don't need `this._handleTabSelected` anymore.

::: mobile/android/chrome/content/browser.js:1221
(Diff revision 4)
>  
>      let selected = "selected" in aParams ? aParams.selected : true;
> -    if (selected)
> +    if (selected) {
> +      let doc = this.selectedBrowser.contentDocument;
> +      if (doc.fullscreenElement) {
> +        aParams.selected = false;

Split this and move this line to before `new Tab`, because `aParams.selected` is used there.

::: mobile/android/chrome/content/browser.js:1322
(Diff revision 4)
>      let doc = this.selectedBrowser.contentDocument;
>      if (doc.fullscreenElement) {
>        // We'll finish the tab selection once the fullscreen transition has ended,
>        // remember the new tab for this.
>        this.fullscreenTransitionTab = aTab;
>        doc.exitFullscreen();

Also, early return here. Otherwise you'll send the select tab message twice.

Comment 40

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104506/diff/4-5/

Comment 41

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

https://reviewboard.mozilla.org/r/104506/#review112232

::: mobile/android/chrome/content/browser.js:434
(Diff revision 5)
>  
>        if (this.fullscreenTransitionTab) {
>          // Tab selection has changed during a fullscreen transition, handle it now.
>          let tab = this.fullscreenTransitionTab;
>          this.fullscreenTransitionTab = null;
> -        this._handleTabSelected(tab);
> +        BrowserApp.selectTab(tab);

`this.selectTab(tab);`

::: mobile/android/chrome/content/browser.js:1218
(Diff revision 5)
> +    }
>      let newTab = new Tab(aURI, aParams);
>  
> +    if (this.selectedBrowser) {
> +       let doc = this.selectedBrowser.contentDocument;
> +       if (doc.fullscreenElement) {

Save the fullscreen state in a variable so you don't check this again.

Comment 42

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104506/diff/5-6/

Comment 43

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8826548 [details]
Bug 1319366 - When fullscreen, defer new tab selection until exiting fullscreen completed.

https://reviewboard.mozilla.org/r/104506/#review112352

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/865aa70feec4
When fullscreen, defer new tab selection until exiting fullscreen completed. r=jchen,sebastian

Comment 45

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/865aa70feec4

Comment 46

[Oana Horvath [:ohorvath]]

Attachment: Screenshot_20170216-174042.png

Verified on Nightly 54.0a1(2017-02-16), with:
- Motorola Nexus 6 (Android 7.0) 
- Huawei Honor 5X (Android 5.1.1).
Opening the new tab will not exit fullscreen, but I've found new issues caused by this:
Steps to reproduce:
1. While in fullscreen mode in the presentation, open the link.
2. Exit fullscreen and go to the tab opened from the presentation.
3. Open the tab drawer and select the presentation tab (or any other tab previously opened, with a functional page).

Expected results: 
2. The page opened from the presentation's link should be loaded in a new tab.
3. The tab selected should show the page contents.

Actual results:
2. A blank page (about:blank) is opened instead of the actual linked page.
3. The tab selected is also blank white, showing the correct url of the page that is in it.
note: Any other tab containing about:blank, created by me, does not cause other tabs to open blank pages. Just the ones that open automatically from the steps I've described.

Comment 47

[Oana Horvath [:ohorvath]]

Attachment: Screenshot_20170216-174052.png

Comment 48

[Nevin Chen(Not active on Bugzilla)]

Hi Carsten
I forgot to add one line.
Would you please help back out this patch so I can submit another review?
Thank you!

Comment 49

[(inactive) Jim Chen [:jchen] [:darchons]]

You can just land another patch on top of the existing patch.

Comment 50

[Carsten Book [:Tomcat]]

(In reply to Nevin Chen [:nechen] from comment #48)
> Hi Carsten
> I forgot to add one line.
> Would you please help back out this patch so I can submit another review?
> Thank you!

Hey nevin,

a clean backout does not work anymore due to other changes i guess, would it possible to submit another patch ? thanks!

Comment 51

[Nevin Chen(Not active on Bugzilla)]

Since bug 1341276 is landed and verified, I guess we can close this bug now?

Comment 52

[Oana Horvath [:ohorvath]]

Hi, I'm marking this as duplicate actually, based on the fact that the fix was made on bug 1341276. Thank you!