Bookmarking a popup doesn't show folder menu

VERIFIED FIXED in Firefox 54

Status

()

defect
P2
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: formicula, Assigned: enndeakin)

Tracking

({regression})

53 Branch
Firefox 55
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox-esr52 unaffected, firefox53- wontfix, firefox54- verified, firefox55- verified)

Details

Attachments

(2 attachments)

Reporter

Description

2 years ago
str
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170413192749

Steps to reproduce:

Visit a website that still shows popup windows (for example profiles on a dating site). Right mouse-click and choose bookmark.


Actual results:

The URL is bookmarked, but the small window that asks you in which folder you want to save it just shows up for a split second, then disappears. It's broken only since Firefox 53. Deleting a bookmark using right-click doesn't work either, because if that window doesn't appear, you can't click the "delete bookmark" button either.


Expected results:

It should show me the small window where I can choose the folder where I want to save my bookmarks or delete one.

Updated

2 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression

Comment 1

2 years ago
Posted file testcase

Comment 2

2 years ago
regression-window
regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5b38cc764e9012ec544cc3b150c35db706e34ced&tochange=d280a7a149bba92ecdb59b9de0a0336c336de889

regressed by: d280a7a149bb	Neil Deakin — Bug 1109868, panels should watch their anchors for position and visibility changes and update accordingly, r=tn
Blocks: 1109868
Component: Untriaged → Bookmarks & History
Flags: needinfo?(enndeakin)
Assignee

Comment 3

2 years ago
I don't see this bug, nor do I see why the bug you suggested causes the issue would affect this.
Flags: needinfo?(enndeakin)

Comment 4

2 years ago
(In reply to Neil Deakin from comment #3)
> I don't see this bug, nor do I see why the bug you suggested causes the
> issue would affect this.

So, in this case, backing out the offending bug is necessary.

Updated

2 years ago
Flags: needinfo?(enndeakin)
When I try the test case I don't see the window to choose which folders to save to. I didn't test this in earlier versions though.   Marco, can you take a look (as triage owner)?
Flags: needinfo?(mak77)
It's too late to fix this in 53 but we could still take a patch for 54.
Neil is the best person to look into this, looks like something changed at the widget level.

Maybe the problem is that Bug 1109868 is working, the open window doesn't have a Star anchor, what we usually did in those cases was to fallback to another anchor, the identity icon:
http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/browser/base/content/browser-places.js#457

If even the identity icon is missing, we anchor to the browser. All the relevant code is up there.
Flags: needinfo?(mak77)
Priority: -- → P2
Assignee

Comment 8

2 years ago
Assignee: nobody → enndeakin
Flags: needinfo?(enndeakin)
Assignee

Updated

2 years ago
Attachment #8864184 - Flags: review?(tnikkel)
Attachment #8864184 - Flags: review?(tnikkel) → review+

Comment 9

2 years ago
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4488424e14ae
when a popup's anchor does not have a frame originally, don't have the popup update its position to follow it, r=tn

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4488424e14ae
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Can this ride the 55 train or should we consider it for uplift to 54?
Flags: needinfo?(enndeakin)
Flags: in-testsuite+
Assignee

Comment 12

2 years ago
This is probably a safe patch.
Flags: needinfo?(enndeakin)
Reporter

Comment 13

2 years ago
Can't you make a 53.0.3? It's an important function that worked in all the older Firefox versions, then all of a sudden was broken in this current one and now you want to take your time and fix it months later?

Am I the only one that noticed this broken function, is there no-one else who reported it on a message board?
chemspill releases (like 53.0.3) are not made for bug fixes, they are only made for security or high impact issues. This is far from being an high impact issue.
I have reproduced this bug with nightly 55.0a1 (2017-04-22) (64-bit) on Manjaro 17 (64bit). I have used (http://www.popuptest.com/popuptest1.html) this site to reproduce this issue.

I can verify that this bug is fixed in Latest Nightly 55.0a1

Build ID 	20170510183715
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170503]
Timothy, can you please nominate this for Beta approval so we can get it into 54 at least? I've confirmed that it grafts cleanly.
Flags: needinfo?(tnikkel)
Comment on attachment 8864184 [details] [diff] [review]
Don't have popups anchored on frameless elements to follow them

Approval Request Comment
[Feature/Bug causing the regression]: bug 1109868 
[User impact if declined]: Bookmarking a popup doesn't show folder menu
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: it was already verified on nightly
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: stops trying to position a popup at an invalid position (0,0) if we can't compute the position
[String changes made/needed]: none
Flags: needinfo?(tnikkel)
Attachment #8864184 - Flags: approval-mozilla-beta?
Comment on attachment 8864184 [details] [diff] [review]
Don't have popups anchored on frameless elements to follow them

Fix a bookmark pop issue and was verified. Beta54+. Should be in 54 beta 9.
Attachment #8864184 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Timothy Nikkel (:tnikkel) from comment #17)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: yes
> [Needs manual test from QE? If yes, steps to reproduce]: it was already
> verified on nightly

This looks like a rather severe issue in terms of user experience, I think we should test it on Beta as well, just to be safe.
Flags: qe-verify+
I have reproduced this issue using Firefox 53 (2017.04.13) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 54.0b9 and 55.0a1 on Win 8.1 x64, Ubuntu 16.04 x64 and Mac OS X 10.10.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1377802
You need to log in before you can comment on or make changes to this bug.