STR: 1. Many Bugzilla page elements have title tooltips. 2. Mouse over the blocked bug 879538 number to see title tooltip "NEW - (core-e10s) e10s back-end tasks tracking bug". RESULT: No title tooltip!

This patch uses the embedding API to send tooltip notifications to the TabParent. I used the nsIXULBrowserWindow interface to actually inform the frontend about the tooltip. I hope that's okay.

This patch displays the tooltip in the frontend.

Comment on attachment 8347545 [details] [diff] [review] tooltip-smaug >+ * Show/hide a tooltip when the mouse hovers over an element in the content >+ * document. >+ */ >+ ShowTooltip(uint32_t x, uint32_t y, nsString tooltip); Please document in which coordinate space these are. >+NS_IMETHODIMP >+TabChild::OnShowTooltip(int32_t aXCoords, int32_t aYCoords, const char16_t *aTipText) * goes with the type This bug needs some tests, especially showing that tooltips are in the correct location even if content is zoomed in or out. With tests added, r+

Or at least make sure manually that the stuff works when zooming

Comment on attachment 8347548 [details] [diff] [review] tooltip-felipe >+ showTooltip: function (x, y, tooltip) { >+ let elt = document.getElementById("remoteBrowserTooltip"); >+ elt.label = tooltip; >+ elt.openPopup(gBrowser.selectedBrowser, "overlap", x, y, false, false, null); >+ }, This isn't quite the same as how other tooltips are shown (they are shown with openPopupAtScreen), so there will be some very subtle differences, but it probably doesn't matter too much for these tooltips. But as commented earlier, you should indicate what kind of coordinates these are. You're using them as if there were relative to the browser window using the chrome's zoom level. Also, it would be good to test right-to-left document and UI as well. >+ >+ hideTooltip: function () { >+ let elt = document.getElementById("remoteBrowserTooltip"); >+ elt.hidePopup(); >+ }, >+ Since this is using the embedding tooltip code, I suspect that this tooltip will have different show/hide behaviour than other tooltips as that code hasn't been updated for a while.

I spent some time changing the content zoom level, and the patch works. However, I only just now realized that you probably want me to test with chrome zooming. However, I don't know how to do that, and Google isn't helping much. Is there a way to change the chrome zoom level? If so, I suspect this patch won't work. Can you think of a way to make it work? I also tried setting the bidi.direction preference to 2. The patch is broken in that case. The child process gives us the correct x,y position assuming that x increases from left to right. But when we try to put the tooltip at that position, it gets switched to the other side, presumably because chrome and the tooltip element itself are in RTL mode. I tried setting the style.direction property of the tooltip to "ltr", but that didn't work. I think I need help with this too. Would it be okay to handle these issues as follow-ups? This patch is a strict improvement over the current situation where tooltips don't work at all in e10s.

Felipe mentioned that setting layout.css.dpi or layout.css.devPixelsPerPx might be a way to change the zoom level in chrome.

I think for testing it should be enough to have content zoomed and check that coordinates are sane.

> I also tried setting the bidi.direction preference to 2. The patch is broken > in that case. The child process gives us the correct x,y position assuming > that x increases from left to right. But when we try to put the tooltip at > that position, it gets switched to the other side, presumably because chrome > and the tooltip element itself are in RTL mode. I tried setting the > style.direction property of the tooltip to "ltr", but that didn't work. I > think I need help with this too. Popups actually use the direction of the anchor to determine this. In your patch this is gBrowser.selectedBrowser. in this case the > > Would it be okay to handle these issues as follow-ups? Since this isn't part of a default build, that is ok.

I added a comment about the coordinate system. I also changed the direction of the browser element to "ltr". Amazingly this worked. I was afraid it would put the actual content back in LTR mode, but it doesn't. If this is too inelegant, I can just take it out and we can fix the text direction issue later. Regarding the test, we're very close to having mochitests run in e10s. I'd rather wait for that than adding e10s-specific tests that will be redundant pretty soon.

Oh, I also tested with layout.css.devPixelsPerPx=2 and everything worked fine.

Comment on attachment 8361417 [details] [diff] [review] tooltip-frontend >+ // Only used in electrolysis. Otherwise popup.xml is used. popup.xml is used in both cases, so that comment is misleading. Just remove it. Same with the comment for hideTooltip. > I added a comment about the coordinate system. I also changed the direction > of the browser element to "ltr". Amazingly this worked. I was afraid it > would put the actual content back in LTR mode, but it doesn't. If this is > too inelegant, I can just take it out and we can fix the text direction > issue later. No, we shouldn't do that. Instead you should just use screen coordinates. For example: elt.openPopupAtScreen(anchor.boxObject.screenX + x, anchor.boxObject.screenY + y, false, null); > I also tried setting the bidi.direction preference to 2. For me, setting that preference breaks the toolbar ui a bit on restart. Instead, to set an english-build ui to be rtl, set 'intl.uidirection.en' to 'rtl'. No restart needed.

Thanks, this works well in LTR and RTL.