Closed
Bug 891194
Opened 11 years ago
Closed 11 years ago
BROKEN_WM_Z_ORDER should not be defined on Mac
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 29
People
(Reporter: markh, Assigned: katiemthom)
References
Details
(Whiteboard: [good first bug][mentor=gavin][qa-])
Attachments
(1 file)
(deleted),
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
The code in the BROKEN_WM_Z_ORDER branch has different semantics (in terms of window ordering) than when it's not defined, as social discovered in bug 874566. It looks like bug 462222 is currently fixing this - once it is done we can just drop this workaround from the module.
Reporter | ||
Comment 1•11 years ago
|
||
Via bug we, the current state seems to be that BROKEN_WM_Z_ORDER should now only be considered true on Linux, but Mac works fine. So in the short term, it might make sense to change the #ifdef in RecentWindow.jsm to reflect this.
Comment 2•11 years ago
|
||
(In reply to Mark Hammond (:markh) from comment #1) > Via bug we Was that supposed to be a bug #? :) If your findings are right, we should re-summarize bug 462222 and adjust BROKEN_WM_Z_ORDER accordingly in a bunch of places.
Reporter | ||
Comment 3•11 years ago
|
||
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2) > (In reply to Mark Hammond (:markh) from comment #1) > > Via bug we > > Was that supposed to be a bug #? :) It was :) Bug 874566.
Comment 4•11 years ago
|
||
re-summarizing this to be about the general problem, now that bug 450576 is WFM.
Updated•11 years ago
|
Whiteboard: [good first bug][mentor=gavin]
Updated•11 years ago
|
Assignee: nobody → katiemthom
Assignee | ||
Comment 5•11 years ago
|
||
This is my first patch--all feedback will be greatly appreciated!
Attachment #8345931 -
Flags: review+
Attachment #8345931 -
Flags: feedback+
Comment 6•11 years ago
|
||
Comment on attachment 8345931 [details] [diff] [review] name.patch Hey Katie, Those review flags should be used to request review - setting them to "?" and entering in your mentor's email address sends the review request. I've taken the liberty of doing that for you, and requesting review from your mentor. Thanks for the patch! -Mike
Attachment #8345931 -
Flags: review?(gavin.sharp)
Attachment #8345931 -
Flags: review+
Attachment #8345931 -
Flags: feedback+
Comment 7•11 years ago
|
||
Comment on attachment 8345931 [details] [diff] [review] name.patch Thanks!
Attachment #8345931 -
Flags: review?(gavin.sharp) → review+
Comment 9•11 years ago
|
||
Comment on attachment 8345931 [details] [diff] [review] name.patch Backed out (https://hg.mozilla.org/integration/fx-team/rev/d3c5304d8011) because this seems to have caused browser_586068-browser_state_interrupted.js to fail on Linux, probably due to a trailing space in the ifdef: >--- a/browser/components/sessionstore/src/SessionStore.jsm >+++ b/browser/components/sessionstore/src/SessionStore.jsm >@@ -81,19 +81,21 @@ const TAB_EVENTS = [ > // Browser events observed. > const BROWSER_EVENTS = [ > "load", "SwapDocShells", "UserTypedValueChanged" > ]; > > // The number of milliseconds in a day > const MS_PER_DAY = 1000.0 * 60.0 * 60.0 * 24.0; > >-#ifndef XP_WIN >+#ifdef XP_UNIX >+#ifndef XP_MACOSX > #define BROKEN_WM_Z_ORDER > #endif >+#endif
Comment 10•11 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #9) > because this seems to have caused > browser_586068-browser_state_interrupted.js to fail on Linux, probably due > to a trailing space in the ifdef: Confirming that this fixes the timeout.
Comment 11•11 years ago
|
||
Relanded with the trailing spaces removed: https://hg.mozilla.org/integration/fx-team/rev/adb909c5d6c3
https://hg.mozilla.org/mozilla-central/rev/adb909c5d6c3
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Updated•11 years ago
|
Whiteboard: [good first bug][mentor=gavin] → [good first bug][mentor=gavin][qa-]
Comment 13•11 years ago
|
||
This may have caused regression bug 924456, see bug 924456 comment 38.
Comment 14•11 years ago
|
||
More "exposed" rather than caused, FWIW.
You need to log in
before you can comment on or make changes to this bug.
Description
•