XUL elements in chrome hidden by content

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
12 years ago
20 days ago

People

(Reporter: tony, Assigned: roc)

Tracking

({regression})

Trunk
x86
All
regression
Points:
---
Bug Flags:
blocking1.9 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

12 years ago
(this probably doesn't belong in safe browsing)

On trunk, XUL elements can't be placed above the browser element.  This breaks safe browsing which places a canvas element and a vbox over the browser element.
(Reporter)

Comment 1

12 years ago
Created attachment 226077 [details]
warning bubble under browser

Comment 2

12 years ago
Sounds like a roc bug.
Is this a recent regression? I thought Fritz had cooked up something to work on trunk.
(Reporter)

Comment 4

12 years ago
(In reply to comment #3)
> Is this a recent regression? I thought Fritz had cooked up something to work on
> trunk.

Yes, it was working a couple weeks ago before the DOM_AGNOSTIC landing.  Now I can't seem to place anything over the browser element.
Would you mind tracking down the regression window?
(Reporter)

Comment 6

12 years ago
(In reply to comment #5)
> Would you mind tracking down the regression window?
> 

With MOZ_CO_DATE=2006-06-17, it works.

With MOZ_CO_DATE=2006-06-18, I get the screen shot in the attachment.
(Reporter)

Comment 7

12 years ago
(In reply to comment #6)
> (In reply to comment #5)
> > Would you mind tracking down the regression window?
> > 
> 
> With MOZ_CO_DATE=2006-06-17, it works.
> 
> With MOZ_CO_DATE=2006-06-18, I get the screen shot in the attachment.
> 

Err, ignore that. That's when I submitted the change to safebrowsing so the bug is visibile.  Let me figure out the actual day.

Updated

12 years ago
Keywords: regression
(Reporter)

Comment 8

12 years ago
Further tests show that this isn't a recent regression (same behavior as on 2006-05-19 build).  It's just different behavior on the bonecho branch than on trunk.  I'll work around it in the safebrowsing code by using the same technique as before (i.e., before bug 329711) by setting browser.collapsed = true.

If the sidebar comes back on trunk, this isn't ideal because we don't get resize events from a collapsed browser.


My test was to run:
var vbox = document.createElement('vbox');                                                    
var cwin = document.getElementById('main-window');                                    
cwin.appendChild(vbox)
vbox.style.position = 'fixed';
vbox.style.height = '400px';
vbox.style.width = '400px';
vbox.style.top = '10px';
vbox.style.left = '10px';
vbox.style.backgroundColor = '#0f0';

In the JS shell in chrome context.  On bonecho, the green square is on top of the browser.  On trunk, it's under the browser.
Keywords: regression
ah, OK. This is just the same underlying issue that's always been there, manifesting in a different way.
Summary: XUL elements can't be placed above the browser element → XUL elements can't be placed above the browser element (safe browsing notification is hidden by content area)
*** Bug 345478 has been marked as a duplicate of this bug. ***
Also occurs on Windows - guessing this is cross OS.

Also, shouldn't this be critical?

-- Ryan Jones
OS: Linux → All
*** Bug 343791 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Severity: normal → major
*** Bug 359158 has been marked as a duplicate of this bug. ***
*** Bug 360805 has been marked as a duplicate of this bug. ***
We should just fix this by changing the phishing protection display to take advantage of the frame display list linkage that roc landed the other day.

Comment 16

12 years ago
Please change summary to 
"Safe browsing notification is hidden by content area"

as it is not currently it is not visible on bugzilla search result
see http://snipurl.com/129n3

IMHO "XUL elements can't be placed above the browser element"
should be a different bug which block this one
and it should be some where under core/layout
(Reporter)

Comment 17

12 years ago
(In reply to comment #15)
> We should just fix this by changing the phishing protection display to take
> advantage of the frame display list linkage that roc landed the other day.

Actually, I think that fixing that bug fixed this bug.  With the latest trunk nightly I'm seeing the warning on top of the browser content area like what is seen on the bonecho branch.

For reference, bug 130078 is the frame display list linkage.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 18

12 years ago
*** Bug 362811 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

12 years ago
Status: RESOLVED → REOPENED
Depends on: 130078
Resolution: WORKSFORME → ---
*** Bug 364563 has been marked as a duplicate of this bug. ***
Status: REOPENED → NEW

Updated

12 years ago
Severity: major → critical
(In reply to comment #17)
> (In reply to comment #15)
> > We should just fix this by changing the phishing protection display to take
> > advantage of the frame display list linkage that roc landed the other day.
> 
> Actually, I think that fixing that bug fixed this bug.  With the latest trunk
> nightly I'm seeing the warning on top of the browser content area like what is
> seen on the bonecho branch.
> 
> For reference, bug 130078 is the frame display list linkage.
> 
Ths is odd.  I still see the issue with current windows nightlies.

Flags: blocking-firefox3?
(Reporter)

Comment 21

12 years ago
(In reply to comment #20)
> (In reply to comment #17)
> > For reference, bug 130078 is the frame display list linkage.
> > 
> Ths is odd.  I still see the issue with current windows nightlies.

The patch for bug 130078 was backed out so this bug exists again.

https://bugzilla.mozilla.org/show_bug.cgi?id=130078#c51

Duplicate of this bug: 370070

Updated

12 years ago
Keywords: regression
(Reporter)

Updated

12 years ago
Duplicate of this bug: 379099
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 beta1
(Reporter)

Comment 24

12 years ago
If we decide to not show the content of phishing (as suggested in bug 380932), then dimming the content won't be a problem.  We can use something like browser.collapsed = true to hide the browser area or show the error in content.
I'd add that fixing this would be a real bonus to extension authors. It must be almost every week that someone comes into #extdev wanting to put something over the top of the webpage.
I can confirm that this bug exists on Linux and XP when running Minefield 3.0a5 build id 2007060104.

Updated

11 years ago

Updated

11 years ago
Duplicate of this bug: 387467

Updated

11 years ago
Target Milestone: Firefox 3 M7 → Firefox 3 M8

Updated

11 years ago
Whiteboard: [needs a plan]
So, one potential solution is to reuse, in less dramatic fashion, the full stop error page we're using for malware.  Due to the infinite redirect technique for evading the blocklist, this actually helps make this more effective.  Assigning to Johnath so we can move forward with this, in the absence of compositor...

moving to M9, M8 will not be a beta.
Assignee: nobody → johnath
Target Milestone: Firefox 3 M8 → Firefox 3 M9
Duplicate of this bug: 397005

Comment 30

11 years ago
Can't the safe browsing popup use the same technique as the new bookmark dialog  (star button) or the identity dialog (Larry) which display properly above the content area, if we can't fix this bug in general?
I think we should morph this bug to cover moving phishing UI to the same "error-page, don't load the content" treatment that we added for malware in bug 380932, as suggested by mconnor in comment 28.  Updating the summary to reflect that more clearly.

By all means, let's move both phishing and malware to the "door hanger" treatment mentioned in bug 385988 when that becomes an option, but for now we should at least be consistent.

The easiest way to do this is to just add/move anti-phishing strings to the netError page as was done with malware, using the same styling as malware, etc.

The downside is that this will offer no click-through, which is something we currently offer for anti-phishing.  A second option would therefore be to add the anti-phishing-specific functionality to netError, and then use CSS to hide/unhide it depending on error.  This means adding some task-specific stuff to an otherwise generic page, I'd like to know what someone like biesi thinks about that, since he reviewed the original malware changes to netError.

The third option is to decide that we are now officially too complicated to piggyback on netError, and just fork it off to something like blacklist.xhtml, which would behave similarly to netError, but have more freedom to include special functionality.

In scenarios 2 & 3, the click-through support would also seem to require some kind of docshell flag to say "please ignore URI classifier hits on the blacklist."  I know Dave is working on that already, but I also know that docshell has apparently run out of bits to attach to new purposes.

Finally, in all cases, the URI classifier code will need to be told about this new UI, and much of the old safebrowsing UI should be removed. 
Status: NEW → ASSIGNED
Summary: XUL elements can't be placed above the browser element (safe browsing notification is hidden by content area) → Anti-phishing UI hidden by content
Whiteboard: [needs a plan]

Comment 32

11 years ago
Johnath, I think you should make a new bug for that workaround / UI change.  This is a real bug with dependencies, a testcase, a regression range, etc.
Summary: Anti-phishing UI hidden by content → Anti-phishing UI (safe browsing notification) hidden by content

Comment 33

11 years ago
<!-- overlay | workaround (Comment #8) -->
<window id="main-window">
  <vbox style="position:fixed; height:400px; width:400px; top:10px; left:10px; background:#0f0">
    <deck style="min-width:100%;min-height:100%;">
      <vbox>
        <label value="???"/>
      </vbox>
    </deck>
  </vbox>
</window>
(In reply to comment #32)
> Johnath, I think you should make a new bug for that workaround / UI change. 
> This is a real bug with dependencies, a testcase, a regression range, etc.

Yeah, I think I agree.  The more I look at this bug, particularly comments like comment 25, the more I think that the anti-phishing aspect is only one piece.  The rest, though, doesn't belong in the Phishing Protection component.

I've opened bug 399233 to track the broken anti-phishing UI, and I'll unmorph this to refer more generically (once again) to chrome content being hidden by browser content.  Based on conversation with Jesse and mconnor, I'll transfer it over to what looks like the right part of Core, and reset the nom flag to ask drivers whether they still think this one should still block FF3.

Assignee: johnath → roc
Status: ASSIGNED → NEW
Component: Phishing Protection → Layout: View Rendering
Flags: blocking-firefox3+
Product: Firefox → Core
QA Contact: phishing.protection → ian
Summary: Anti-phishing UI (safe browsing notification) hidden by content → XUL elements in chrome hidden by content
Target Milestone: Firefox 3 M9 → ---
Whoops - blocking flag got lost when it jumped from Firefox to Core, re-nom'ng.  This may or may not need to be a blocker now, given that bug 399233 has been nom'd for the anti-phishing aspects.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
> vbox.style.position = 'fixed';

Ah, I finally figured out the story here!

The story is that position:fixed elements used to get native widgets created for them. On trunk they don't.

Try using "overflow:auto" instead.
Actually I'm going to mark this WONTFIX, since what happened is we broke a hack workaround for the chrome-over-content problem. Other hack workarounds exist and should be used instead.
Status: NEW → RESOLVED
Last Resolved: 12 years ago11 years ago
Resolution: --- → WONTFIX
Your suggestion about using "overflow:auto" works, BUT there is a problem with the borders of the elements in Firefox 3.
Can you please explain some of these "hack workarounds" to get this working as expected.

I'm using the following code:

var vbox = document.createElement('vbox');                                      
var cwin = document.getElementById('main-window');                              
cwin.appendChild(vbox)
vbox.style.position='fixed';
vbox.style.overflow= 'auto';
vbox.style.height = '400px';
vbox.style.width = '400px';
vbox.style.right = '10px';
vbox.style.top= '10px';
vbox.style.border='10px';
vbox.style.borderColor='black';
vbox.style.borderStyle='solid';
vbox.style.backgroundColor = '#0f0';
Created attachment 308795 [details]
without overflow:auto, firefox 3 all versions
Created attachment 308796 [details]
with overflow:auto, only borders hidden by content, firefox 3 all versions
This bug resurrected with 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091028 Minefield/3.7a1pre

Comment 42

9 years ago
Actually, this 'bug' resurrected already with Firefox 3.6 IMHO.

Looks like XUL 'panel' is the (only?) way to go in Firefox 3.6

In Firefox itself 'panel' is used for various elements that appear on top of 'anything', like bookmark panel, site identity popup...
http://mxr.mozilla.org/mozilla1.9.1/source/browser/base/content/browser.xul#121
http://mxr.mozilla.org/mozilla1.9.2/source/browser/themes/pinstripe/browser/browser.css#997
This bug is fixed on current trunk!!!!!!! YAY!
Created attachment 502392 [details]
Firefox 4.0b9pre

Update picture for firefox 4.
Resolution: WONTFIX → FIXED
Component: Layout: View Rendering → Layout: Web Painting
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.