Tour Overlay does not navigate to sync page when in a new tab

VERIFIED FIXED in Firefox 57

Status

()

Firefox
New Tab Page
P2
normal
VERIFIED FIXED
9 months ago
7 months ago

People

(Reporter: JW_SoftvisionQA, Assigned: Gabor Krizsanits (INACTIVE))

Tracking

({qawanted, regression})

57 Branch
Firefox 57
qawanted, regression
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57+ verified)

Details

(Whiteboard: [photon-onboarding])

Attachments

(1 attachment)

(Reporter)

Description

9 months ago
STR:
1. Start the tour as a update user
2. Open a new tab and the tour overlay
3. Select sync and type in a valid email
4. Click Next

Expected:
Sync page appears 

Actual:
the overlay closes

This only happens the first time in a new tab. 

See video below:
https://jwilliams-softvision.tinytake.com/sf/MTg5NTE2MV82MDYzMjE0

Comment 1

9 months ago
hi,

could you please double check if Fx56 is affected as well?
status-firefox56: --- → ?
Keywords: qawanted
Priority: P3 → P2

Updated

9 months ago
Flags: qe-verify+
QA Contact: jwilliams
(Reporter)

Comment 2

9 months ago
This is not reproducible on 56. Only reproducible on 57.
status-firefox55: --- → unaffected
status-firefox56: ? → unaffected
status-firefox-esr52: --- → unaffected
After doing bisecting, this is a regression caused by bug 1376895.

:krizsa Do you have any idea how we can fix this issue?

What it do here by UITour.jsm is to call browser.loadURI() to navigate to about:accounts inside about:newtab.[1]

[1]http://searchfox.org/mozilla-central/source/browser/components/uitour/UITour.jsm#588
Depends on: 1376895
Flags: needinfo?(gkrizsanits)
Keywords: regression
(Assignee)

Comment 4

9 months ago
(In reply to KM Lee [:rexboy] from comment #3)
> After doing bisecting, this is a regression caused by bug 1376895.
> 
> :krizsa Do you have any idea how we can fix this issue?

I think I need some more info about what's happening exactly, I'm not familiar with this code at all.

> 
> What it do here by UITour.jsm is to call browser.loadURI() to navigate to
> about:accounts inside about:newtab.[1]
> 
> [1]http://searchfox.org/mozilla-central/source/browser/components/uitour/
> UITour.jsm#588

Are you sure that browser.loadURI is called? That should just work I think. If it gets called and fails, where does it fail? There is an exception on the console btw:
Error: No user is logged in  FxAccountsStorage.jsm:209:13 does anyone know what that means or what might cause that?

Bug 1376895 makes sure that once we leave about:newtab (navigating away) we always try to find a new process for it, but I guess about:accounts should be running in the parent process, no? In that case it should not change much.

Maybe it's worth checking if adding an uri != "about:accounts" check here: http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/browser/base/content/browser.js#1093 fixes the problem.
Flags: needinfo?(gkrizsanits)

Comment 5

9 months ago
Tracked for 57, this bug (among others) will track the "Onboarding spec change" work planned for early Beta57
tracking-firefox57: --- → +
Adding uri != "about:accounts" doesn't seem to work.

I noticed that LoadInOtherProcess() was called twice after calling browser.loadURI(). Seems the first call redirects them to about:accounts correctly, but then the second call brought it back to about:newtab. The second call came from RedirectLoad().

If I checkout earlier version, the second call doesn't raise.

I dumped the call stack from inside LoadInOtherProcess():

===
[1st call]

Stack:
LoadInOtherProcess@chrome://browser/content/browser.js:1174:10
_loadURIWithFlags@chrome://browser/content/browser.js:1142:7
loadURIWithFlags@chrome://browser/content/tabbrowser.xml:8326:13
loadURI/<@chrome://global/content/bindings/browser.xml:114:15
_wrapURIChangeCall@chrome://global/content/bindings/browser.xml:48:15
loadURI@chrome://global/content/bindings/browser.xml:113:13
onPageEvent@resource:///modules/UITour.jsm:588:9
@file:///Users/rexboy/firefox/obj-x86_64-apple-darwin15.6.0/dist/Nightly.app/Contents/Resources/browser/components/nsBrowserGlue.js:2969:3

loadOptions parameter:
{"uri":"about:accounts?action=signup&entrypoint=uitour&email=xxxxxx%40gmail.com","triggeringPrincipal":null,"flags":0,"referrer":null,"referrerPolicy":0,"remoteType":null}

===
[2nd call] (which didn't happen before)

loadOptions:
{"uri":"about:newtab","flags":0,"referrer":"about:newtab","triggeringPrincipal":"ZT4OTT7kRfqycpfCC8AeuAAAAAAAAAAAwAAAAAAAAEYB4NodcC97EdOM0ABgsPwUo5IHOlRteE8wkTq4cYEyCMYAAAAABWFib3V0AAAABm5ld3RhYgAAAAAAAAAA","reloadInFreshProcess":false}

Stack:
LoadInOtherProcess@chrome://browser/content/browser.js:1174:10
RedirectLoad@chrome://browser/content/browser.js:1205:5
===

So seems we got an unnecessary RedirceLoad()... If I just skip LoadInOtherProcess() in RedirectLoad, about:accounts shows correctly.
Does that give you some insight of cause?
Flags: needinfo?(gkrizsanits)
(Assignee)

Comment 7

9 months ago
(In reply to KM Lee [:rexboy] from comment #6)
> [2nd call] (which didn't happen before)
> 
> loadOptions:
> {"uri":"about:newtab","flags":0,"referrer":"about:newtab",
> "triggeringPrincipal":
> "ZT4OTT7kRfqycpfCC8AeuAAAAAAAAAAAwAAAAAAAAEYB4NodcC97EdOM0ABgsPwUo5IHOlRteE8w
> kTq4cYEyCMYAAAAABWFib3V0AAAABm5ld3RhYgAAAAAAAAAA","reloadInFreshProcess":
> false}
> 
> Stack:
> LoadInOtherProcess@chrome://browser/content/browser.js:1174:10
> RedirectLoad@chrome://browser/content/browser.js:1205:5
> ===
> 
> So seems we got an unnecessary RedirceLoad()... If I just skip
> LoadInOtherProcess() in RedirectLoad, about:accounts shows correctly.
> Does that give you some insight of cause?

That's weird, that should not happen. And in a reduced case it does not, so something else must be going on here within about:newtab. What is weird that the uri is about:newtab (as well as the referrer) I have no idea where does that request come from. Based on the stack the request comes from the child process, could you get the child side stack for this as well (where the related "Browser:LoadURI" message is sent). That is the message we should block, but I don't know where it comes from exactly, it makes no sense to me that about:newtab is trying to load about:newtab in the middle of a navigation toward about:accounts.
Flags: needinfo?(gkrizsanits)
(Assignee)

Comment 8

9 months ago
Also, in a custom mc build sync is not available from about:newtab, is there a way to make it available? Then I could help with the debugging...
Flags: needinfo?(rexboy)
Oh yeah. Thanks for your help.
Sync tour in version 57 is only visible for an updated user.
You can change browser.onboarding.tour-type to "update" to see it.

I'll try to give you the child side's stack soon.
Flags: needinfo?(rexboy) → needinfo?(gkrizsanits)
And by the way you need to open 2 new tabs to see it after changing the pref because the first one is preloaded.
The Browser:LoadURI message came from [1] where the stack is:

redirectLoad@resource:///modules/E10SUtils.jsm:264:10
shouldLoadURI@chrome://browser/content/tab-content.js:718:10

If I delete the if() in[2], the additional redirect won't get called.

I'm not quite familiar with the code here, does that mean we're trying to open a newly preload about:newtab, but rather than load it in background, it got loaded accidentally in the current tab?

[1] http://searchfox.org/mozilla-central/source/browser/modules/E10SUtils.jsm#264
[2] http://searchfox.org/mozilla-central/source/browser/modules/E10SUtils.jsm#251
(Assignee)

Comment 12

8 months ago
Created attachment 8907022 [details] [diff] [review]
the default tour overlay form submit navigation should be cancelled. v1

So the C++ side of the stack is: [1]

It seems like the HMTL form on submit initiates a navigation. I guess cancelling the default behavior for the event prevents this issue. Are you the right person to review this patch? Could you confirm that this patch fixes the bug?


[1]:
#01: nsDocShell::OnLinkClickSync(nsIContent*, nsIURI*, char16_t const*, nsAString const&, nsIInputStream*, nsIInputStream*, bool, nsIDocShell**, nsIRequest**, nsIPrincipal*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7972e09]
#02: non-virtual thunk to nsDocShell::OnLinkClickSync(nsIContent*, nsIURI*, char16_t const*, nsAString const&, nsIInputStream*, nsIInputStream*, bool, nsIDocShell**, nsIRequest**, nsIPrincipal*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x79732b1]
#03: mozilla::dom::HTMLFormElement::SubmitSubmission(mozilla::dom::HTMLFormSubmission*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3c21c6e]
#04: mozilla::dom::HTMLFormElement::DoSubmit(mozilla::WidgetEvent*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3c212cb]
#05: mozilla::dom::HTMLFormElement::DoSubmitOrReset(mozilla::WidgetEvent*, mozilla::EventMessage)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3c2028d]
#06: mozilla::dom::HTMLFormElement::PostHandleEvent(mozilla::EventChainPostVisitor&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3c20f73]
#07: mozilla::EventTargetChainItem::PostHandleEvent(mozilla::EventChainPostVisitor&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a72f0b]
#08: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a73184]
#09: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a733ce]
#10: mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a74ce9]
#11: mozilla::PresShell::HandleDOMEventWithTarget(nsIContent*, mozilla::WidgetEvent*, nsEventStatus*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e6402]
#12: mozilla::dom::HTMLButtonElement::PostHandleEvent(mozilla::EventChainPostVisitor&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3bfed9a]
#13: mozilla::EventTargetChainItem::PostHandleEvent(mozilla::EventChainPostVisitor&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a72f0b]
#14: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a73184]
#15: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a733ce]
#16: mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a74ce9]
#17: mozilla::PresShell::DispatchEventToDOM(mozilla::WidgetEvent*, nsEventStatus*, nsPresShellEventCB*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e623c]
#18: mozilla::PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e4302]
#19: mozilla::PresShell::HandleEventWithTarget(mozilla::WidgetEvent*, nsIFrame*, nsIContent*, nsEventStatus*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e53c2]
#20: mozilla::EventStateManager::InitAndDispatchClickEvent(mozilla::WidgetMouseEvent*, nsEventStatus*, mozilla::EventMessage, nsIPresShell*, nsIContent*, AutoWeakFrame, bool)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a3ce53]
#21: mozilla::EventStateManager::CheckForAndDispatchClick(mozilla::WidgetMouseEvent*, nsEventStatus*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a38ff4]
#22: mozilla::EventStateManager::PostHandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsEventStatus*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x3a3780b]
#23: mozilla::PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e43c9]
#24: mozilla::PresShell::HandlePositionedEvent(nsIFrame*, mozilla::WidgetGUIEvent*, nsEventStatus*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e509b]
#25: mozilla::PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x52e2a0c]
#26: nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4d70584]
#27: nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4d702ec]
#28: mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4dabecf]
#29: mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent(mozilla::WidgetGUIEvent&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1e7902c]
#30: mozilla::dom::TabChild::DispatchWidgetEventViaAPZ(mozilla::WidgetGUIEvent&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4867f35]
#31: mozilla::dom::TabChild::RecvRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long long const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x48679f3]
#32: mozilla::dom::TabChild::RecvNormalPriorityRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long long const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4867fbd]
#33: non-virtual thunk to mozilla::dom::TabChild::RecvNormalPriorityRealMouseButtonEvent(mozilla::WidgetMouseEvent const&, mozilla::layers::ScrollableLayerGuid const&, unsigned long long const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x486800c]
#34: mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x12cb2f3]
#35: mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x1420387]
#36: mozilla::dom::ContentChild::OnMessageReceived(IPC::Message const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x480e1c7]
#37: mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc3f63]
#38: mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc239b]
#39: mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc2f44]
#40: mozilla::ipc::MessageChannel::MessageTask::Run()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc36d8]
#41: mozilla::SchedulerGroup::Runnable::Run()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x212602]
#42: nsThread::ProcessNextEvent(bool, bool*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x243406]
#43: NS_ProcessNextEvent(nsIThread*, bool)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x246cdc]
#44: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc845f]
#45: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc90b3]
#46: MessageLoop::RunInternal()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba3255]
#47: MessageLoop::RunHandler()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba31b5]
#48: MessageLoop::Run()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba315d]
#49: nsBaseAppShell::Run()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4dd3623]
#50: nsAppShell::Run()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x4e70ae2]
#51: XRE_RunAppShell()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x81dd140]
#52: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xcc8f11]
#53: MessageLoop::RunInternal()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba3255]
#54: MessageLoop::RunHandler()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba31b5]
#55: MessageLoop::Run()[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0xba315d]
#56: XRE_InitChildProcess(int, char**, XREChildData const*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x81dc782]
#57: mozilla::BootstrapImpl::XRE_InitChildProcess(int, char**, XREChildData const*)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/XUL +0x81e9ac7]
#58: content_process_main(mozilla::Bootstrap*, int, char**)[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/plugin-container.app/Contents/MacOS/plugin-container +0x180a]
#59: main[/Users/gkrizsanits/Documents/development/mozilla-central/obj-x86_64-apple-darwin16.6.0/dist/NightlyDebug.app/Contents/MacOS/plugin-container.app/Contents/MacOS/plugin-container +0x18c5]
Assignee: nobody → gkrizsanits
Flags: needinfo?(gkrizsanits)
Attachment #8907022 - Flags: review?(rexboy)
Comment on attachment 8907022 [details] [diff] [review]
the default tour overlay form submit navigation should be cancelled. v1

Review of attachment 8907022 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for creating the patch!
It works well for me, just the preventdefault() need to be postponed until submit event to confirm form validation show messages correctly.

I'm still wondering whether the inconsistency between about:home and about:newtab is a good behavior, but this patch is reasonable enough for this issue.

::: browser/extensions/onboarding/content/onboarding-tour-agent.js
@@ +51,5 @@
>      case "onboarding-tour-singlesearch-button":
>        Mozilla.UITour.showMenu("urlbar");
>        break;
>      case "onboarding-tour-sync-button":
> +      evt.preventDefault();

We need to do this in submit event in order to let form validation show correspond message. Please just put these code at the end of file instead:

document.getElementById("onboarding-overlay").addEventListener("submit", e => {
  e.preventDefault();
});
Attachment #8907022 - Flags: review?(rexboy) → review+

Comment 14

8 months ago
Comment on attachment 8907022 [details] [diff] [review]
the default tour overlay form submit navigation should be cancelled. v1

Review of attachment 8907022 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/extensions/onboarding/content/onboarding-tour-agent.js
@@ +51,5 @@
>      case "onboarding-tour-singlesearch-button":
>        Mozilla.UITour.showMenu("urlbar");
>        break;
>      case "onboarding-tour-sync-button":
> +      evt.preventDefault();

Drive-by:
Since we have done `let overlay = document.getElementById("onboarding-overlay");` at [1] already we should just do `overlay.addEventListener("submit", e => e.preventDefault());` at [2], thanks.

[1] http://searchfox.org/mozilla-central/rev/e76c0fee79a34a3612740b33508276f4b37c3ee8/browser/extensions/onboarding/content/onboarding-tour-agent.js#74
[2] http://searchfox.org/mozilla-central/rev/e76c0fee79a34a3612740b33508276f4b37c3ee8/browser/extensions/onboarding/content/onboarding-tour-agent.js#75

Updated

8 months ago
Status: NEW → ASSIGNED

Comment 15

8 months ago
Pushed by gkrizsanits@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c250263488b
Prevent tour overlay form to confuse the process selecting algorithm. r=rexboy
https://hg.mozilla.org/mozilla-central/rev/9c250263488b
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
(Reporter)

Comment 17

8 months ago
I have verified that this issue has been fixed on today's nightly.
Status: RESOLVED → VERIFIED
I can confirm the intended behavior is respected on beta. I verified using Fx 57.0b7 on Windows 10 x64, Ubuntu 14.04 LTS and macOS X 10.12.6.
status-firefox57: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.