950564 - Complex text input panel for plugin should be positioned at bottom-left of focused plugin

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, enhancement)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Similar to bug 950559, on Mac, composition string of IME is displayed on complex text input panel when plugin has focus. The panel should be positioned at bottom-left of the focused plugin because it's nearer than current fixed position (bottom-most of the screen, screen-width).

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: screenshot

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: ComplexTextInputPanel should be positioned to bottom-left of the focused plugin

Similar to bug 950559, the panel is aligned to window if the plugin is clipped.

I want Kimura-san to agree the behavior. I'll request the code review to Steven after that.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

tryserver build:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=0ef2594de29c

Comment 4

[Masatoshi Kimura [:emk]]

Comment on attachment 8347900 [details] [diff] [review]
ComplexTextInputPanel should be positioned to bottom-left of the focused plugin

I can't really review a patch for Mac, but it would be OK as long as it is consistent with a windowless Flash movie on other platforms.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Comment on attachment 8347900 [details] [diff] [review]
ComplexTextInputPanel should be positioned to bottom-left of the focused plugin

Thank you, Kimura-san.

Steven, could you review this? As I mentioned in this patch, I'm not familiar with Retina display and I don't have it. So, this patch hasn't been tested on Retina display. Therefore, I'm not sure if the min-width pref works well.

FYI: I confirmed that this patch doesn't cause bustage on 10.6.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Masayuki, I can test your patch on OS X 10.7 through 10.9, using Retina and non-Retina displays.  I should be able to get to this in the next few days.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8347900 [details] [diff] [review]
ComplexTextInputPanel should be positioned to bottom-left of the focused plugin

Masayuki, this looks fine to me.

One small nit, though:  Please add a comment above adjustTo: that describes exactly what adjustments you're making -- for example something like "This places the text input panel fully onscreen and below the lower left corner of the focused plugin."

But I still haven't tested it, because I have nothing to test with.  My favorite example of a Flash movie that accepts text input has disappeared from the web (http://www.playercore.com/bugfiles/146162/AddReturnHTML.html from bug 357670 comment #25).  Please post an example of a plugin I can test with (one that accepts text input, and which causes us to display a text input panel when entering text in, say, Kotoeri Hiragana mode).

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Thank you, Steven.

Kimura-san created a great testcase here:
http://emk.name/test/swftxt.html

Comment 9

[Steven Michaud [:smichaud] (Retired)]

> http://emk.name/test/swftxt.html

Thanks!

I used this to test on OS X 10.7 through 10.9, with and without HiDPI mode.  I had no problems at all.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Thank you very much!

https://hg.mozilla.org/integration/mozilla-inbound/rev/be7c246b13b7

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/be7c246b13b7

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

> +                           std::min(viewRect.size.width, minWidth),

Oops... This should be std::max()! I'll post follow up patch next week :-(

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: part.2 Fix complex text input panel width as focused plugin width if it's not too narrow

Fixing the nits...

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: Screenshot of complex text input panel with large plugin

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: Screenshot of complex text input panel with small plugin

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Comment on attachment 8350976 [details] [diff] [review]
part.2 Fix complex text input panel width as focused plugin width if it's not too narrow

I'm sorry, this fixes very stupid mistake.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

FYI: test builds:
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=bf15266dce36

Comment 18

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 8350976 [details] [diff] [review]
part.2 Fix complex text input panel width as focused plugin width if it's not too narrow

:-)

Sorry I didn't catch this.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Thanks!
https://hg.mozilla.org/integration/mozilla-inbound/rev/60e35dc29ad9

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/60e35dc29ad9