Closed Bug 1106906 Opened 5 years ago Closed 5 years ago

The default button in modal dialogs on OSX 10.10 is rendered empty initially

Categories

(Core :: Widget: Cocoa, defect, major)

x86
macOS
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla38
Tracking Status
firefox35 --- unaffected
firefox36 + verified
firefox37 --- verified
firefox38 --- verified

People

(Reporter: ehsan, Assigned: mstange)

References

(Blocks 1 open bug)

Details

(Keywords: regression, reproducible, testcase)

Attachments

(3 files)

Attached image Screenshot
See the screenshot.  The rendering pops in as soon as I hover the button.
Can you provide STR?
Flags: needinfo?(ehsan.akhgari)
Keywords: steps-wanted
Duplicate of this bug: 1106372
STR:

1. Load http://people.mozilla.org/~eakhgari/1106906.html.
2. Press Submit Query.
3. Reload.
Flags: needinfo?(ehsan.akhgari)
Keywords: steps-wantedtestcase
Duplicate of this bug: 1107497
Last good revision: 469fdce88cb7
First bad revision: 7f34fc3870ba
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=469fdce88cb7&tochange=7f34fc3870ba

Looks like this was caused by the fix to Bug 1072342. Blocking that and needinfo'ing Neal Deakin, the committer of the fix.
Blocks: 1072342
Flags: needinfo?(enndeakin)
Sounds like this is the sheet issue I mentioned in bug 1072342 comment 27. I had missed the fact that :-moz-window-inactive is not just used by plugins but also by default buttons.
[Tracking Requested - why for this release]:

This regression causes modal dialogs on OS X to have no text on the default button, e.g. "OK".
Markus, would you have a fix for this?
Flags: needinfo?(enndeakin) → needinfo?(mstange)
Important regression. Tracking.
Duplicate of this bug: 1100401
I'll take a look.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
When a sheet opens, it's first painted with inactive widgets, and when the sheet opening animation has finished, the sheet gets an activate event and is supposed to repaint with active widgets.

The problem here is that we have two different ways of determining the sheet window's activeness state, and they disagree with each other. Method one is checking frame->GetContent()->OwnerDoc()->GetDocumentState().HasState(NS_DOCUMENT_STATE_WINDOW_INACTIVE), which is what nsDisplayThemedBackground does when it checks whether widgets need to be invalidated. Method two is checking [nativeSheetWindow isKeyWindow], which is what nsNativeThemeCocoa does when determining whether the button should be active (blue) or inactive (white).

The thing that changed with bug 1072342 is that the sheet window starts out active (in terms of document state, but not in terms of [nativeSheetWindow isKeyWindow] state), so when it gets the activate event after the animation, the button is not invalidated.
Neil, do you have a preferred way of making the sheet start out inactive, or any other ideas on how this should work?
Flags: needinfo?(enndeakin)
Duplicate of this bug: 1108673
Duplicate of this bug: 1110815
I've never been able to reproduce this. Maybe it's only noticeable on certain os versions?

I think nsFocusManager::IsParentActivated for parent processes is wrong. It should always be returning false, otherwise it will return true if any window is active. It shouldn't need to initialize anything in the parent process.
Flags: needinfo?(enndeakin)
Duplicate of this bug: 1111144
Attached image screenshot
There is a similar bug in Firefox 34 (STR: Open Firefox with a new profile), is this related or another bug?
Blocks: 1100401
(In reply to Sören Hentzschel from comment #18)
> Created attachment 8536587 [details]
> screenshot
> 
> There is a similar bug in Firefox 34 (STR: Open Firefox with a new profile),
> is this related or another bug?

bug 1100401 - related but not quite identical.
Duplicate of this bug: 1114906
Duplicate of this bug: 1116594
I'm bumping the importance of this bug up to "major." This is a huge UI regression, as users won't be able to see all choices in a modal unless they hover over the hidden one.
Severity: normal → major
Duplicate of this bug: 1118242
Duplicate of this bug: 1120055
Attached patch patchSplinter Review
Sorry for the delay here everybody. This patch turned out way simpler than I was anticipating.
Attachment #8548347 - Flags: review?(enndeakin)
Duplicate of this bug: 1118196
Duplicate of this bug: 1121803
Duplicate of this bug: 1123493
Comment on attachment 8548347 [details] [diff] [review]
patch

Neil's on PTO, maybe smaug can help.
Attachment #8548347 - Flags: review?(bugs)
Comment on attachment 8548347 [details] [diff] [review]
patch

Yeah, this should bring the old behavior on non-e10s / parent process.
Attachment #8548347 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/6f4d9e4adf7c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Markus, could you fill the uplift request to aurora & beta? Thanks
Flags: needinfo?(mstange)
Comment on attachment 8548347 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1072342
[User impact if declined]: invisible text on sheet default buttons
[Describe test coverage new/current, TreeHerder]: none for this particular issue, but good enough existing coverage to ensure that we don't break much else
[Risks and why]: low, it's a small and low impact patch
[String/UUID change made/needed]: none
Flags: needinfo?(mstange)
Attachment #8548347 - Flags: review?(enndeakin)
Attachment #8548347 - Flags: approval-mozilla-beta?
Attachment #8548347 - Flags: approval-mozilla-aurora?
Attachment #8548347 - Flags: approval-mozilla-beta?
Attachment #8548347 - Flags: approval-mozilla-beta+
Attachment #8548347 - Flags: approval-mozilla-aurora?
Attachment #8548347 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
QA Contact: catalin.varga
Depends on: 1127950
Verified as fixed using the following environment:

FF36.0b9
Build Id:20150212154903
OS: Mac Os X 10.10
Verified as fixed with Firefox 37 beta 1 and Nightly 38.0a1 under Mac OS X 10.10.
Status: RESOLVED → VERIFIED
See Also: → 1147350
You need to log in before you can comment on or make changes to this bug.