Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Firefox asks for confirmation when leaving an exercise that is synchronized #467

Closed
yurug opened this issue Mar 4, 2022 · 1 comment
Assignees

Comments

@yurug
Copy link
Collaborator

yurug commented Mar 4, 2022

Related issue(s) or PR(s):

Bug description

Erik noticed:

Minor issue: when the tab is already synchronized, I think the confirmation dialog This page is asking you to confirm that you want to leave - data you have entered may not be saved. should not appear, while it appears unconditionally(?)

and Yann tried by several means to fix this without success. The documented way that should ensure confirmation deactivation is to delete the returnValue property of the HTML event... but Firefox ignores such deletion.

@yurug yurug added the kind: bug label Mar 4, 2022
@erikmd erikmd added this to the learn-ocaml 0.14.0 milestone Mar 4, 2022
@erikmd erikmd self-assigned this Mar 4, 2022
erikmd added a commit that referenced this issue Mar 4, 2022


This hack is needed to address <#467>
because [Dom.handler] <https://ocsigen.org/js_of_ocaml/latest/api/js_of_ocaml/Js_of_ocaml/Dom/index.html#val-handler>
appears to be incompatible with "onbeforeunload" and Firefox.

* Namely, the semantics of [Dom.handler] requires to "return true" to tell [Dom.handler] that we want to unload normally,
* but with some browsers such as Firefox we should actually "return undefined" (better than "return null" BTW),
* but then, "undefined" is seen as "false", implying [Dom.handler] will call "preventDefault",
* end_of_the_story.

Credits: the working code from this commit reuses a patch by @leunam217,
pfitaxel@15780b5

See also: https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload#example
@erikmd
Copy link
Member

erikmd commented Mar 5, 2022

So, this issue will be resolved by #372 but just to keep some explanation/details for future reference:

The first implementation version in #372 was working with Chrome:

  let onbeforeunload ?(win = Dom_html.window) f =
    win##.onbeforeunload := Dom_html.handler (fun ev ->
                                let (status, propagate) = f ev in
                                if status then (
                                   Js.bool propagate
                                ) else
                                  (
                                    Js.(Unsafe.eval_string "undefined");
                                  )
                             )

But Dom.handler appears to be incompatible with "onbeforeunload" and Firefox.

  • Namely, the semantics of Dom.handler requires to "return true" to tell Dom.handler that we want to unload normally,
  • but with some browsers such as Firefox we should actually "return undefined" (better than "return null" BTW),
  • but then, "undefined" is seen as "false", implying Dom.handler will call "preventDefault",
  • end_of_the_story.

A sketch of an onbeforeunload-specific handler could be:

  (* Copy of private [Dom.window_event] *)
  let window_event' () : 'a #Dom.event Js.t = Js.Unsafe.pure_js_expr "event" in
  (* Fixed copy of [Dom.handler], working for onbeforeunload
     even for Firefox *)
  let handler_onbeforeunload :
        ((#Js_of_ocaml.Dom_html.event as 'b) Js_of_ocaml__.Js.t ->
         bool Js_of_ocaml__.Js.t) ->
        (* ('a, 'b Js_of_ocaml__.Js.t) Js_of_ocaml.Dom_html.event_listener = *)
        ('a, 'b Js_of_ocaml__.Js.t -> bool Js.t) Js.meth_callback Js.opt =
    fun f ->
  Js.some
    (Js.Unsafe.callback (fun e ->
         (* depending on the internet explorer version, e can be null or undefined. *)
         if not (Js.Opt.test (Js.some e))
         then (
           let e = window_event' () in
           let res = f e in
           if not (Js.to_bool res) then
             (e##.returnValue := res; res)
           else
             Js.Unsafe.pure_js_expr "undefined"
         ) else (
           let res = f e in
           if not (Js.to_bool res) then
             ((Js.Unsafe.coerce e)##preventDefault (* ; res *))
           else
             (Js.Unsafe.delete (Js.Unsafe.coerce e) "returnValue";
              Js.Unsafe.pure_js_expr "undefined")))) in
   *)

Anyway, the current hack is quite simpler:

  let prompt_before_unload () : unit =
    Js.Unsafe.js_expr "window.onbeforeunload = function(e) {e.preventDefault(); return false;}" in
  let resume_before_unload () : unit =
    Js.Unsafe.js_expr "window.onbeforeunload = null" in
  let () =
    Ace.register_sync_observer ace (fun sync ->
        if not sync then prompt_before_unload ()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants