Closed
Bug 607252
Opened 14 years ago
Closed 14 years ago
Doorhanger arrow is within content area, so easy to simulate by a webpage
Categories
(Toolkit :: XUL Widgets, defect)
Toolkit
XUL Widgets
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: alfredkayser, Assigned: Margaret)
References
Details
(Whiteboard: [doorhanger])
Attachments
(5 files, 2 obsolete files)
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
faaborg
:
ui-review+
|
Details |
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•14 years ago
|
||
Reporter | ||
Updated•14 years ago
|
Updated•14 years ago
|
Assignee: enndeakin → nobody
Component: XP Toolkit/Widgets: XUL → XUL Widgets
Product: Core → Toolkit
QA Contact: xptoolkit.xul → xul.widgets
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.
Updated•14 years ago
|
Assignee: nobody → margaret.leibovic
blocking2.0: ? → final+
Assignee | ||
Comment 3•14 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•14 years ago
|
||
I'm marking this as a dupe because bug 606343 will fix the problem.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Comment 5•14 years ago
|
||
Did you mean a different bug? I don't plan on fixing this in bug 606343.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee | ||
Comment 6•14 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•14 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•14 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•14 years ago
|
Whiteboard: [should be fixed by 606343]
Comment 9•14 years ago
|
||
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•14 years ago
|
||
I'm duping this bug to bug 606343, since the patch for that bug fixed the problem.
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → DUPLICATE
Whiteboard: [should be fixed by 606343]
Comment 11•14 years ago
|
||
Not fixed for me with small icons and the bookmarks toolbar hidden.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee | ||
Comment 12•14 years ago
|
||
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•14 years ago
|
||
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•14 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 15•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
Attachment #496087 -
Flags: review?(dao) → review?(enndeakin)
Comment 17•14 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•14 years ago
|
||
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•14 years ago
|
||
Attachment #496175 -
Flags: ui-review?(shorlander)
Updated•14 years ago
|
Attachment #496171 -
Flags: review?(enndeakin) → review+
Assignee | ||
Updated•14 years ago
|
Attachment #496175 -
Flags: ui-review?(shorlander) → ui-review?(faaborg)
Updated•14 years ago
|
Attachment #496175 -
Flags: ui-review?(faaborg) → ui-review+
Assignee | ||
Comment 20•14 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•