Show the leading Section headline text in PageInfoBubbleView.

Show the leading Section headline text in PageInfoBubbleView.

Ensure that the leading headline text is shown on layout.
Selecting a [0,0] range moves the cursor to the leading text edge.
Textfield implementations then pan to show the cursor position.

Verified on local builds CrOS and Windows (with win/views textfields).

BUG=140273
TEST=Page info bubbles show the leading section text on Win and CrOS.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149967

[msw, 2012-08-03 06:34:56]

Hey Peter, please take a look, thanks!

BTW, PageInfoBubble code is pretty bad (recreates Sections on every animation
step!?!?), hopefully WebsiteSettings will be a lot nicer :)

[Peter Kasting, 2012-08-03 19:20:37]

https://chromiumcodereview.appspot.com/10826126/diff/2001/chrome/browser/ui/v...
File chrome/browser/ui/views/page_info_bubble_view.cc (right):

https://chromiumcodereview.appspot.com/10826126/diff/2001/chrome/browser/ui/v...
chrome/browser/ui/views/page_info_bubble_view.cc:433:
headline_label_->SelectRange(ui::Range());
I don't think this is the right fix.  Instead, the headline label needs to
either break across lines, or elide.  Either will remove the need for this and
look less broken.

It would be nice if the width of the bubble took the headline text size into
account, so that if it wasn't unreasonably long we'd show it all untruncated.

[msw, 2012-08-03 20:10:19]

https://chromiumcodereview.appspot.com/10826126/diff/2001/chrome/browser/ui/v...
File chrome/browser/ui/views/page_info_bubble_view.cc (right):

https://chromiumcodereview.appspot.com/10826126/diff/2001/chrome/browser/ui/v...
chrome/browser/ui/views/page_info_bubble_view.cc:433:
headline_label_->SelectRange(ui::Range());
On 2012/08/03 19:20:37, Peter Kasting wrote:
> I don't think this is the right fix.  Instead, the headline label needs to
> either break across lines, or elide.  Either will remove the need for this and
> look less broken.
> 
> It would be nice if the width of the bubble took the headline text size into
> account, so that if it wasn't unreasonably long we'd show it all untruncated.

This fixes the regression and brings us to parity with M19. I agree with your
thoughts for a better long-term fix, but I think we should land this safe, small
change and request merge to M21.

(FYI, we'd need to add views::Textfield multi-line or eliding/fading support for
your suggestions. A better interim fix to show the whole string might be to make
the Views PageInfoBubble wider, as it is on other platforms).

[Peter Kasting, 2012-08-03 20:12:46]

Why is this thing a textfield and not a label?  It doesn't look to the user as
if it's a textfield and it doesn't seem critical that it be selectable.

Being a label would give us inherent support for eliding or linebreaking.

[msw, 2012-08-03 20:30:06]

+ASvitkine for additional history. Apparently it's intentionally a textfield in
order to support copying the text that's there.

I think we could do a lot of things to improve the bubble here, but it makes
sense to at least resolve this regression (initially showing trailing text
instead of the leading text) before pursuing larger changes.

Additionally, the PageInfoBubble should be replaced by the WebsiteSettings
bubble in M23 afaik, so it may not be worth a lot of time to fix.

[Peter Kasting, 2012-08-03 20:35:14]

LGTM I guess as a bandaid.

Really not a fan of making this a textfield -- copying within bubbles is
strange.  It looks to me in dev channel as if this text is now non-copyable
anyway.  I'd probably just make this a label for M22 and set its width to the
correct size so it auto-elides, and be done with it.  But whatever.

[msw, 2012-08-03 20:52:40]

On 2012/08/03 20:35:14, Peter Kasting wrote:
> LGTM I guess as a bandaid.
> 
> Really not a fan of making this a textfield -- copying within bubbles is
> strange.  It looks to me in dev channel as if this text is now non-copyable
> anyway.  I'd probably just make this a label for M22 and set its width to the
> correct size so it auto-elides, and be done with it.  But whatever.

Agreed, but it's been a textfield for a while, and this really is just a
band-aid for the regression. WebsiteSettings should take your thoughts into
consideration (+CC MarkusHeintz). 

File a bug if you think this should be a Label for M22 (multi-line or
eliding/fading?, how can we support copying if that's a security/privacy issue?
should the other platforms also switch to not be selectable/copyable (or offer
some alternate copy approach) and should they also switch to use multi-line?)
it's not entirely obvious/straightforward, afaict.

[commit-bot: I haz the power, 2012-08-03 20:52:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10826126/2001

[commit-bot: I haz the power, 2012-08-03 21:07:01]

Try job failure for 10826126-2001 (retry) on mac for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

[commit-bot: I haz the power, 2012-08-03 22:02:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/10826126/2001

[commit-bot: I haz the power, 2012-08-03 23:25:54]

Change committed as 149967