Closed
Bug 1325880
Opened 8 years ago
Closed 8 years ago
[RTL] When a link is close to Firefox's bottom, hovering its tooltip doesn't make the tooltip go to the left
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
VERIFIED
FIXED
Firefox 55
People
(Reporter: itiel_yn8, Assigned: Gijs)
References
Details
(Keywords: rtl)
Attachments
(5 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
text/x-review-board-request
|
dao
:
review+
gchang
:
approval-mozilla-aurora+
gchang
:
approval-mozilla-beta+
|
Details |
STR:
1. Download & install any RTL build of Firefox
2. Open a page where you know there's inside a link located *very* close to the browser's bottom right (in the attached screenshot it's about:newtab)
3. Hover that link very closly to the bottom right, so you'd also hover the URL tooltip
4. Instead of going to the left side of the browser, the tooltip goes only a few pixels left.
See attached screenshots.
The same STR works fine on LTR builds.
I've tried pinpointing the exact regression (though I'm not sure this is even a regression) and Firefox 39 also suffers from this bug (older versions may also have the same symptoms, didn't check it yet).
Comment 3•8 years ago
|
||
Comment 4•8 years ago
|
||
Please note that the statusbar/tooltip border is broken when hovering the bottom right link from the testcase above.
Updated•8 years ago
|
Component: General → Layout: Text
Product: Firefox → Core
The issue can be observed also when loading a page and hovering the tooltip below (the one that says for example "Loading www.google.com").
Updated•8 years ago
|
Assignee | ||
Updated•8 years ago
|
Component: Layout: Text → Tabbed Browser
Product: Core → Firefox
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to ItielMaN from comment #2)
> The same STR works fine on LTR builds.
> I've tried pinpointing the exact regression (though I'm not sure this is
> even a regression) and Firefox 39 also suffers from this bug (older versions
> may also have the same symptoms, didn't check it yet).
Is this a regression?
Flags: needinfo?(itiel_yn8)
(In reply to :Gijs (gone until 3 jan) from comment #6)
> (In reply to ItielMaN from comment #2)
> > The same STR works fine on LTR builds.
> > I've tried pinpointing the exact regression (though I'm not sure this is
> > even a regression) and Firefox 39 also suffers from this bug (older versions
> > may also have the same symptoms, didn't check it yet).
>
> Is this a regression?
Can you tell me what version/build was this feature first implemented so I'd check?
Currently I'm blindly checking versions.
Flags: needinfo?(itiel_yn8)
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to ItielMaN from comment #7)
> (In reply to :Gijs (gone until 3 jan) from comment #6)
> > (In reply to ItielMaN from comment #2)
> > > The same STR works fine on LTR builds.
> > > I've tried pinpointing the exact regression (though I'm not sure this is
> > > even a regression) and Firefox 39 also suffers from this bug (older versions
> > > may also have the same symptoms, didn't check it yet).
> >
> > Is this a regression?
>
> Can you tell me what version/build was this feature first implemented so I'd
> check?
> Currently I'm blindly checking versions.
bug 628654, Firefox 4.
It's totally possible this isn't a regression and has always been broken. I just don't know. If it *is* a regression, it'd be useful to know that.
Flags: needinfo?(itiel_yn8)
Comment 9•8 years ago
|
||
regression-window |
Executed command: ./mozregression --pref intl.uidirection.en-US:rtl --arg="https://bug1325880.bmoattachments.org/attachment.cgi?id=8821928"
9:21.03 INFO: Narrowed nightly regression window from [2013-09-11, 2013-09-13] (2 days) to [2013-09-12, 2013-09-13] (1 days) (~0 steps left)
9:21.03 INFO: Got as far as we can go bisecting nightlies...
9:21.03 INFO: Last good revision: a4e9c9c9dbf9 (2013-09-12)
9:21.03 INFO: First bad revision: b9029b1de410 (2013-09-13)
9:21.03 INFO: Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a4e9c9c9dbf9&tochange=b9029b1de410
I guess this is a regression of bug 821687.
Flags: needinfo?(itiel_yn8)
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Tomer Cohen :tomer from comment #9)
> Executed command: ./mozregression --pref intl.uidirection.en-US:rtl
> --arg="https://bug1325880.bmoattachments.org/attachment.cgi?id=8821928"
>
> 9:21.03 INFO: Narrowed nightly regression window from [2013-09-11,
> 2013-09-13] (2 days) to [2013-09-12, 2013-09-13] (1 days) (~0 steps left)
> 9:21.03 INFO: Got as far as we can go bisecting nightlies...
> 9:21.03 INFO: Last good revision: a4e9c9c9dbf9 (2013-09-12)
> 9:21.03 INFO: First bad revision: b9029b1de410 (2013-09-13)
> 9:21.03 INFO: Pushlog:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=a4e9c9c9dbf9&tochange=b9029b1de410
>
> I guess this is a regression of bug 821687.
That looks very plausible, yes. Thanks for figuring this out!
Blocks: 821687
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(gijskruitbosch+bugs)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
Assignee | ||
Comment 12•8 years ago
|
||
The offset-inline-start/end changes aren't strictly necessary, I suspect, but they simplify the selectors involved and IMO the behaviour is easier to reason about with these selectors than with the previous ones.
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8845372 [details]
Bug 1325880 - fix RTL issue with statuspanel display,
https://reviewboard.mozilla.org/r/118578/#review120486
::: browser/base/content/browser.css:867
(Diff revision 1)
> position: fixed;
> margin-top: -3em;
> max-width: calc(100% - 5px);
> pointer-events: none;
> + offset-inline-start: 0;
> + offset-inline-end: auto;
The above additions shouldn't be necessary. (The statuspanel[mirrror] changes are fine.)
Attachment #8845372 -
Flags: review?(dao+bmo) → review+
Comment hidden (mozreview-request) |
Comment 15•8 years ago
|
||
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bc67ce5424a2
fix RTL issue with statuspanel display, r=dao
Comment 16•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Reporter | ||
Comment 17•8 years ago
|
||
Working great on latest Nightly.
Marking as VERIFIED FIXED.
Status: RESOLVED → VERIFIED
Updated•8 years ago
|
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
status-firefox-esr52:
--- → affected
Comment 18•8 years ago
|
||
Comment on attachment 8845372 [details]
Bug 1325880 - fix RTL issue with statuspanel display,
Approval Request Comment
[User impact if declined]: target link doesn't display well on RTL builds
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes, verified by ItielMan
[Needs manual test from QE? If yes, steps to reproduce]: testcase included
[Is the change risky?]: No
[Why is the change risky/not risky?]: Minor CSS change
Attachment #8845372 -
Flags: approval-mozilla-beta?
Attachment #8845372 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8845372 [details]
Bug 1325880 - fix RTL issue with statuspanel display,
(In reply to Tomer Cohen :tomer from comment #18)
> Comment on attachment 8845372 [details]
> Bug 1325880 - fix RTL issue with statuspanel display,
>
> Approval Request Comment
> [User impact if declined]: target link doesn't display well on RTL builds
> [Is this code covered by automated tests?]: No
> [Has the fix been verified in Nightly?]: Yes, verified by ItielMan
> [Needs manual test from QE? If yes, steps to reproduce]: testcase included
> [Is the change risky?]: No
> [Why is the change risky/not risky?]: Minor CSS change
Please don't request uplift on patches that you didn't write or review without checking with the author/reviewer, esp. if you're not super familiar with the code. If you think something deserves uplift, ask the author/reviewer (maybe with a needinfo).
As it is, I thought this request made sense but was lacking details. It seems you've just deleted some bits of the form? Here's an amended uplift request:
Approval Request Comment
[Feature/Bug causing the regression]: old regression in the behaviour of the status tooltips that replaced the status panel
[User impact if declined]: in RTL, links on the bottom-right-hand-side of the viewport can be obscured by the status panel when the user hovers over them (rather than the panel moving out of the way as it's supposed to). More generally, the status panel continues obscuring whatever is there if it appears based on keyboard focus (despite the position of the mouse)
[Is this code covered by automated tests?]: not to my knowledge, certainly not for this particularity.
[Has the fix been verified in Nightly?]: yes, see comment
[Needs manual test from QE? If yes, steps to reproduce]:
comment #0 has one, but these might be a little more explicit rather than QE having to find a page with links in the bottom corner of the screen:
1. open Firefox on an RTL build (or use the "force RTL" add-on or flip the intl.uidirection pref (on recent builds))
2. open about:newtab
3. focus the search field
4. hit the tab key until you hit a link (ie tile)
5. move the mouse to the bottom right corner without hovering any other tiles
Expected: status tooltip with the URL moves out of the way
Pre-patch: the tooltip shifts a tiny bit but not to the other side of the viewport
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's a minor CSS change that shouldn't impact anything besides this panel, where issues are quickly spotted.
[String changes made/needed]: no
Comment 20•8 years ago
|
||
Comment on attachment 8845372 [details]
Bug 1325880 - fix RTL issue with statuspanel display,
Fix RTL issue with statuspanel display and was verified. Beta53+ & Aurora54+.
Attachment #8845372 -
Flags: approval-mozilla-beta?
Attachment #8845372 -
Flags: approval-mozilla-beta+
Attachment #8845372 -
Flags: approval-mozilla-aurora?
Attachment #8845372 -
Flags: approval-mozilla-aurora+
Comment 21•8 years ago
|
||
bugherder uplift |
Comment 22•8 years ago
|
||
bugherder uplift |
Updated•8 years ago
|
Flags: qe-verify+
Comment 23•8 years ago
|
||
Reproduced on Firefox 52 on Windows 10 x64 using the en-US build switched to RTL.
Confirming the fix on Windows 10 x64, Mac OS X 10.11.6 and Ubuntu 14.04 x64 using the following Firefox builds:
* latest 54.0a2 Aurora, build ID: 20170314004020
* 53 beta 2, build ID: 20170313154936.
You need to log in
before you can comment on or make changes to this bug.
Description
•