The default bug view has changed. See this FAQ.

[10.7] Give search fields in window chrome the new on-chrome style on 10.7

RESOLVED FIXED in mozilla9

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

({polish})

Trunk
mozilla9
All
Mac OS X
polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 556251 [details] [diff] [review]
v1

(In reply to Stephen Horlander from bug 681974 comment #0)
> Textfields on unified toolbars in Lion have a slightly different appearance
> consisting of:
> 
> - Lighter border
> - Subtle dark inner gradient
> - Subtle inner-shadow differences
Attachment #556251 - Flags: review?(joshmoz)
Summary: Give search fields in window chrome the new on-chrome style on 10.7 → [10.7] Give search fields in window chrome the new on-chrome style on 10.7

Comment 1

6 years ago
Comment on attachment 556251 [details] [diff] [review]
v1

Review of attachment 556251 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/src/cocoa/nsNativeThemeCocoa.mm
@@ +234,5 @@
> +{
> +  mContext = aContext;
> +}
> +
> +static BOOL IsWindowChrome(nsIFrame* aFrame)

I'd expect this to query the frame's content for chrome (privileged) status. This doesn't do that, it assumes certain elements are chrome. Theoretically that could be wrong, and it won't return true for all privileged content. Can you come up with a less confusing name?

@@ +262,5 @@
> +  // the searchfield is on top of of window chrome. This function is called on
> +  // 10.7 during drawing in order to determine which style to use.
> +  for (nsIFrame* frame = mContext; frame; frame = frame->GetParent()) {
> +    if (IsWindowChrome(frame))
> +      return YES;

I prefer {} around all blocks.

@@ +897,5 @@
>    DrawCellWithSnapping(cell, cgContext, inBoxRect, searchFieldSettings,
>                         VerticalAlignFactor(aFrame), mCellDrawView,
>                         IsFrameRTL(aFrame));
>  
> +  [cell setContext:nsnull];

Works for me but you might want to document this ptr management where you declare the class in order to avoid usage that leads to a dangling pointer.
Attachment #556251 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 2

6 years ago
Renamed IsWindowChrome to IsToolbarStyleContainer and added a comment.

http://hg.mozilla.org/integration/mozilla-inbound/rev/9943ca23ec8a
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/9943ca23ec8a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.