1.44 KB, patch
|Details | Diff | Splinter Review|
58 bytes, text/x-review-board-request
Apparently nobody has run a debug build on gtk2 for several months, because bug 1217515 set the pref to autodetect touch event support, and touch event autodetection isn't supported on gtk2, only gtk3, so when we switched beta back to gtk2, we get debug test logs like https://treeherder.mozilla.org/logviewer.html#?job_id=783451&repo=mozilla-beta with a trillion NS_WARNINGS until we overflow the maximum log size.
Tracking as it is a critical issue
Ah, thanks to http://mxr.mozilla.org/mozilla-central/source/testing/profiles/prefs_general.js#49 we only get it in test suites which use a profile, but don't use prefs based on that, which seems to be jsreftests and e10s-reftests.
Make that "everything that's run by the reftest harness", including crashtests and non-e10s reftests, but those two have enough log size headroom to be able to absorb the trillions of warnings without log overflow.
Assignee: nobody → philringnalda
Status: NEW → ASSIGNED
Attachment #8717302 - Flags: review?(sledru)
Comment on attachment 8717302 [details] [diff] [review] Set the pref to not spew thanks
Attachment #8717302 - Flags: review?(sledru) → review+
I think we should actually just drop the warning from dom/events/TouchEvent.cpp and set it to 2 (or just not override it) in the testing harnesses.
Comment on attachment 8717302 [details] [diff] [review] Set the pref to not spew https://hg.mozilla.org/releases/mozilla-beta/rev/74179c50c9d1
Attachment #8717302 - Flags: checkin+
Not spewing certainly seems like the right thing to do, my very first thought about this was "but, doesn't that make gtk2 debug builds completely unusable?" I was just aiming to get a patch landed that would let us run jsreftests and e10s reftests and thus reopen the tree, but we've still got trunk and aurora to fix, and perhaps beta to refix.
(In reply to Kartikaya Gupta (email:firstname.lastname@example.org) from comment #6) > I think we should actually just drop the warning from > dom/events/TouchEvent.cpp Do you mean just for GTK2, or in general?
In general. For platforms that don't support touch events we can just "autodetect" false. Just like how for b2g/android we "autodetect" true at https://dxr.mozilla.org/mozilla-central/rev/584870f1cbc5d060a57e147ce249f736956e2b62/dom/events/TouchEvent.cpp#185
I thought the point of the warning was that we had some platforms where we *could* auto-detect, it just isn't implemented, but perhaps there are no such platforms.
Just to be clear - with the change to logging, this stops being a blocker?
Right, that wasn't an accidental change of severity. It is no longer a blocker for 45, it will next become a blocker when we don't do anything on aurora, merge aurora to beta, and then decide yet again to punt on shipping gtk3 at 7pm minutes before going to build with a beta.
Assignee: nobody → bugmail.mozilla
Component: Widget: Gtk → DOM: Events
Review commit: https://reviewboard.mozilla.org/r/34789/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/34789/
Attachment #8718898 - Flags: review?(botond)
Since this also called so often, I tried making the code use gfxPrefs instead of Preferences, but that crashed on startup because it gets invoked before gfxPlatform is initialized and so before gfxPrefs is set up.
Comment on attachment 8718898 [details] MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond https://reviewboard.mozilla.org/r/34789/#review31415 Please add a comment at https://dxr.mozilla.org/mozilla-central/rev/576a6dcde5b68c2ea45324ed5ce1dabb7d833d09/modules/libpref/init/all.js#4644 saying "auto-detect is currently only supported on Windows and GTK3".
Attachment #8718898 - Flags: review?(botond) → review+
Does this need uplifting to 46? I'm assuming not but I don't know what the state of gtk3 is. Please flip status-firefox46 to wontfix if we don't need it uplifted.
I would assume it does need uplifting, unless you want to have local (or third-party automation) gtk2 builds be unable to sanely run reftests and then have sudden permaorange if relman yet again decides at the last second to not ship gtk3 like they have twice already.
Comment on attachment 8718898 [details] MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond Approval Request Comment [Feature/regressing bug #]: bug 1217515 [User impact if declined]: tests possibly failing on debug builds due to log overflowing with NS_WARNING. No user-visible impact since NS_WARNING does nothing in opt builds. See philor's above comment. [Describe test coverage new/current, TreeHerder]: covered by automated tests [Risks and why]: low risk, just removing a NS_WARNING [String/UUID change made/needed]: none
Attachment #8718898 - Flags: approval-mozilla-beta?
Comment on attachment 8718898 [details] MozReview Request: Bug 1246854 - Remove unnecessary warning. r?botond Match the 45 release, taking it. Should be in 46 beta 5
Attachment #8718898 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.