Doorhanger arrow is within content area, so easy to simulate by a webpage

VERIFIED FIXED

Status

()

Toolkit
XUL Widgets
--
major
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Alfred Kayser, Assigned: Margaret)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [doorhanger])

Attachments

(5 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
Created attachment 486015 [details]
Doorhanger in default theme

See attached images.

How to reproduce:
Disabled bookmarks toolbar, use default theme in Windows (but other may have same problem).
Drag&drop an extension into the webpage to install it, or
go to addons.mozilla.org to install an extension.

Result:
The arrow/doorhanger panel is completely within the content space.
This allows websites to simulate this panel.

Expected result: 
The arrow is on top of Firefox chrome.
(Reporter)

Comment 1

8 years ago
Created attachment 486016 [details]
How it should look (using Nautipolis theme)
(Reporter)

Updated

8 years ago
Assignee: nobody → enndeakin
Blocks: 554937
blocking2.0: --- → ?

Updated

8 years ago
Depends on: 606343

Updated

8 years ago
Assignee: enndeakin → nobody
Component: XP Toolkit/Widgets: XUL → XUL Widgets
Product: Core → Toolkit
QA Contact: xptoolkit.xul → xul.widgets

Comment 2

8 years ago
You are right at the moment, but this is due to a bug: the arrow is misaligned: https://bugzilla.mozilla.org/show_bug.cgi?id=606343 
The arrow is supposed to be over the chrome as in your second screenshot. When the bug I mentioned is fixed, this should no longer be a problem.
Assignee: nobody → margaret.leibovic
blocking2.0: ? → final+
(Assignee)

Comment 3

8 years ago
I think we just need to fix bug 606343. We could use a negative margin in the browser theme code for a temporary fix, but that won't fix the underlying alignment problem.
(Assignee)

Comment 4

8 years ago
I'm marking this as a dupe because bug 606343 will fix the problem.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 606343

Comment 5

8 years ago
Did you mean a different bug? I don't plan on fixing this in bug 606343.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Updated

8 years ago
Blocks: 597557
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> Did you mean a different bug? I don't plan on fixing this in bug 606343.

Once bug 606343 is fixed this won't be a problem anymore, since the arrow will point to the anchor and overlap with the browser chrome. I saw this when I applied the patch from bug 606343, so I think you are fixing the problem :)

I'll leave this bug open for now, in case bug 606343 somehow doesn't manage to fix it, but I think it will.

Comment 7

8 years ago
OK, a specific case of this was caused by the extra padding on Windows, which I've needed to remove to fix an issue in the patch for bug 606343.

Is this bug referring to more that this though, on other platforms?
(Assignee)

Comment 8

8 years ago
(In reply to comment #7)
> OK, a specific case of this was caused by the extra padding on Windows, which
> I've needed to remove to fix an issue in the patch for bug 606343.
> 
> Is this bug referring to more that this though, on other platforms?

I don't think this problem occurs on other platforms. When I was working on my other styling patches I experimented with removing that padding to fix this problem, but Dão and I decided to leave it alone in the hopes that it would be fixed by bug 606343.

With that change, are any of the panels still completely in the content area?
(Assignee)

Updated

8 years ago
Whiteboard: [should be fixed by 606343]
I think we should go ahead and close this, unless you're wanting to use this bug to address a particular remaining style glitch.

Doorhangers are not being used for the input of sensitive info, so the most a spoofed doorhanger can do is maybe get a user to click an unprivledged button in content. Given the widespread fake "your computer has a virus" OS dialogish tricks, I don't think doorhangers are a particularly interesting vector for spoofing anyway.
(Assignee)

Comment 10

8 years ago
I'm duping this bug to bug 606343, since the patch for that bug fixed the problem.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → DUPLICATE
Whiteboard: [should be fixed by 606343]
Duplicate of bug: 606343
Not fixed for me with small icons and the bookmarks toolbar hidden.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 12

8 years ago
Created attachment 495968 [details]
arrow point on Windows vs OSX (small icons enabled)

The tip of the arrow points to a different part of the anchor icon on Windows and OSX. I think we should try to get the Windows popup to do what the OSX popup is doing.

Enn, would you know how to adjust this? I tried adding a negative margin to the panel in popup.css, but that caused the arrow to disappear sometimes. It seemed to alternately appear and disappear every time I opened the panel. Is there a bug on that?
(Assignee)

Comment 13

8 years ago
Created attachment 495985 [details] [diff] [review]
patch

This patch moves the arrow point closer to the center of the anchor. I don't know if this is a hacky approach, since pinstripe doesn't need these margins to achieve the same effect, but it gets the job done.
Attachment #495985 - Flags: review?(dao)
(Assignee)

Comment 14

8 years ago
It seems like the reason for the difference on Windows is just that the arrow image includes more pixels of shadow by the point of the arrow, so it looks farther away from the anchor icon. Therefore, I think that this negative margin is the best bet to fix that (or we could change the images, but I think we still want that shadow to be there).
Comment on attachment 495985 [details] [diff] [review]
patch

Need to do the same for side=left/right. And the margin should probably be set on the panel rather than arrow?
Attachment #495985 - Flags: review?(dao) → review-
(Assignee)

Comment 16

8 years ago
Created attachment 496087 [details] [diff] [review]
patch v2

Here's a patch that includes the same negative margins for left/right arrows.

I first tried setting the margin on the panel itself, but that caused the arrow to disappear. I agree that seems like a more sensible place for the margin, but the panels behave correctly when the margin is set on the arrows.
Attachment #495985 - Attachment is obsolete: true
Attachment #496087 - Flags: review?(dao)

Updated

8 years ago
Attachment #496087 - Flags: review?(dao) → review?(enndeakin)

Comment 17

8 years ago
Comment on attachment 496087 [details] [diff] [review]
patch v2

Does this affect both Windows 7 and XP?
Attachment #496087 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 18

8 years ago
Created attachment 496171 [details] [diff] [review]
patch v3

The old patch affected Windows 7 and XP the same, but testing on XP, I realized that the arrow was 1px too high, so I changed the margin to -4px for that platform. I'll upload a screenshot for ui-review, but here's the revised patch.
Attachment #496087 - Attachment is obsolete: true
Attachment #496171 - Flags: review?(enndeakin)
(Assignee)

Comment 19

8 years ago
Created attachment 496175 [details]
screenshot with new patch (osx arrow included for comparison)
Attachment #496175 - Flags: ui-review?(shorlander)

Updated

8 years ago
Attachment #496171 - Flags: review?(enndeakin) → review+
(Assignee)

Updated

8 years ago
Attachment #496175 - Flags: ui-review?(shorlander) → ui-review?(faaborg)
Attachment #496175 - Flags: ui-review?(faaborg) → ui-review+
(Assignee)

Updated

8 years ago
Blocks: 615476
(Assignee)

Comment 20

8 years ago
http://hg.mozilla.org/mozilla-central/rev/90f6cd0ff3e7
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Whiteboard: [doorhanger]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.