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

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Markus Stange [:mstange]]

Attachment: 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

Comment 1

[Josh Aas]

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.

Comment 2

[Markus Stange [:mstange]]

Renamed IsWindowChrome to IsToolbarStyleContainer and added a comment.

http://hg.mozilla.org/integration/mozilla-inbound/rev/9943ca23ec8a

Comment 3

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/9943ca23ec8a