Opened 5 years ago

Closed 5 years ago

#14893 closed defect (fixed)

GTK errors due to use of deprecated -moz styles in about:tor and elsewhere?

Reported by: mikeperry Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: TorBrowserTeam201502
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

policy suspects that our use of -moz prefixed styles (especially deprecated ones) may be causing the stream of GTK errors on Linux:

07:52 < policy> anybody observes tons of "Gtk-CRITICAL **: IA__gtk_clipboard_set_with_data: assertion 'targets != NULL' failed"
07:53 < policy> and "GLib-CRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed" at start and yet many warns
07:57 < policy> but at least one can be fixed https://bugzilla.mozilla.org/show_bug.cgi?id=827876
07:58 < policy> "defined attributes for -moz-user-select instead of user-select"
07:58 < policy> probably not only user-select

Child Tickets

Change History (6)

comment:1 Changed 5 years ago by arma

Our friend says "if clipboard filled by outside stuff then no errors on newident"

https://bugzilla.mozilla.org/show_bug.cgi?id=815952 looks like the Firefox bug that is causing our message when we trigger it.

Suggested workaround is "probably solution to clear it explicitly on newident by torbutton. that way the firefox bug when it fails to clear it won't show up? if nothing to clear then ffox will not trigger this bug."

This workaround might fail though -- it depends whether *our* action of clearing the clipboard nsIClipboard triggers the bug when we try to preemptively clear it.

comment:2 Changed 5 years ago by cypherpunks

Possible workaround for one of those asserts:

   torbutton_log(3, "New Identity: Disabling JS");
   torbutton_disable_all_js();
 
+  /* This workaround for gtk when browser tries to pass NULL
+   * and triggering assert error:
+   * Gtk-CRITICAL **: IA__gtk_clipboard_set_with_data: assertion 'targets != NULL' failed
+   *
+   * XXX: https://bugzilla.mozilla.org/show_bug.cgi?id=815952 for make it
+   *      configurable for next after ESR-31 */
+  try {
+      var nsIClipboard = Components.classes["@mozilla.org/widget/clipboard;1"].
+                                    getService(Components.interfaces.nsIClipboard); 
+
+      /* Clear clipboard by empty string if previous data was stored in private mode */ 
+      if (nsIClipboard.hasDataMatchingFlavors(["application/x-moz-private-browsing"],
+                                              1, nsIClipboard.kGlobalClipboard)) {
+        var clipboard = Components.classes["@mozilla.org/widget/clipboardhelper;1"].
+                                   getService(Components.interfaces.nsIClipboardHelper);
+        clipboard.copyString("");
+      }
+  } catch(e) {
+      torbutton_log(3, "Exception on clipboard secure clear: "+e);
+  }
+
   m_tb_prefs.setBoolPref("browser.zoom.siteSpecific",
                          !m_tb_prefs.getBoolPref("browser.zoom.siteSpecific"));
   m_tb_prefs.setBoolPref("browser.zoom.siteSpecific",

comment:3 Changed 5 years ago by cypherpunks

Workaround above changes behavior for non-gtk platform which can to handle transferable with null data flavor and to clear clipboard. It's possible to modify proposed patch and to change behavior on gtk platform instead, like this:

      if (nsIClipboard.hasDataMatchingFlavors(["application/x-moz-private-browsing"],
                                              1, nsIClipboard.kGlobalClipboard)) {
          var xferable = Components.classes["@mozilla.org/widget/transferable;1"]
                                   .createInstance(Components.interfaces.nsITransferable);
          xferable.addDataFlavor("");
          nsIClipboard.setData(xferable, null, nsIClipboard.kGlobalClipboard);
      }

It should to clear clipboard and to keep the same behavior for all platforms. Need testing.
Another solution to modify browser code (nsClipboardPrivacyHandler::Observe) to pass not null flavor?

comment:4 Changed 5 years ago by cypherpunks

ESR-38 planned for May, it's less than 3 month till bug will be auto-magically fixed. Not sure anyone able to implement fix for non-explode bug so fast, assert crash is bad but not as long as it's auto-magically recoverable by code. So lets to close this bug as fixed (it's really fixed just not for current ESR).
Another assert and warns is well known for vanilla firefox too.

comment:5 in reply to:  4 Changed 5 years ago by arma

Replying to cypherpunks:

ESR-38 planned for May, it's less than 3 month till bug will be auto-magically fixed. Not sure anyone able to implement fix for non-explode bug so fast, assert crash is bad but not as long as it's auto-magically recoverable by code. So lets to close this bug as fixed (it's really fixed just not for current ESR).
Another assert and warns is well known for vanilla firefox too.

Sounds like a plausible plan to me.

comment:6 Changed 5 years ago by mikeperry

Resolution: fixed
Status: newclosed

Ok, I am in agreement. We wait for FF38-ESR for this one. Thanks for investigating, cypherpunks.

Note: See TracTickets for help on using tickets.