Closed Bug 1364366 Opened 7 years ago Closed 1 year ago

browser_action.browser_style doesn't set margin

Categories

(WebExtensions :: Frontend, defect, P3)

Product:
WebExtensions
Component:
Frontend
Version:
52 Branch
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: sebastian.noack, Unassigned)

References

Details

(Whiteboard: [triaged][browserAction])

Attachments

(1 file)

screen.png
4.42 KB, image/png
Details 
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

Steps to reproduce:

Create and install a WebExtension that uses a browser action popup using the browser_style option.

This is the extension I had the problem with, but it uses a workaround now:
https://github.com/snoack/mypass/tree/master/extension


Actual results:

The browser_style option exists, so that developers that opt to use it don't have to bother about styling their browser action popup, while ensuring a (reasonable) consistent appearance. However, by default the margin (and padding, of the <html> and <body> element) will be 0px, which makes the popup look quite squashed, so that you to manually implement a margin (which is no longer consistent with other extensions).


Expected results:

Firefox should set a reasonable margin (Chrome uses 8px on the <html> element), for browser action popups, at least when using (but not necessarily limited to) the browser_style option.
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit
Hi Blake,  Kris suggested to ask you about this bug.
Component: WebExtensions: Untriaged → WebExtensions: Frontend
Flags: needinfo?(bwu)
(In reply to :shell escalante from comment #1)
> Hi Blake,  Kris suggested to ask you about this bug.
I am more familiar with media, not WebExtensions. I am sorry that you may get wrong Blake. :-)
Flags: needinfo?(bwu) → needinfo?(sescalante)
sorry bwu - meant to ask blake kaplan.

Hi Blake Kaplan :) - Kris suggested to ask you about this bug.
Flags: needinfo?(sescalante) → needinfo?(mrbkap)
Sorry, wrong Blake.
Flags: needinfo?(mrbkap) → needinfo?(bwinton)
So many Blakes…

I think this is more a question for Markus, although he seems to be on PTO, so perhaps Amin could answer whether we want padding on the default browser style or not…
Flags: needinfo?(bwinton) → needinfo?(aalhazwani)
Apologies but between a public holiday and a sick leave I missed this one. Passing this to Markus as he's back to PTO.
Flags: needinfo?(aalhazwani) → needinfo?(mjaritz)
Priority: -- → P3
Whiteboard: [triaged][browserAction]
Sebastian, what was the problem you ran into with your extension? A screenshot would really help.

From what I read hear, in general browser_style is intended to provide developers with an easy way to get to extensions that look like Firefox UI. I would not just add a default margin, but aim for providing default styles for HTML elements that make it easy for devs like you to build something that looks like Firefox UI, without needing to create a lot of css. (Not working too well right now.)
For an easy way to get to, let's say a Firefox-menu-style list, a general margin would be counter-productive as our list elements extend all the way to the sides. 
I will take a note to review/refine the default style for browser_style.

Sidenote: For extensions without browser_style the margin is set to 0 to allow extensions to be able to fill the full panel, and for extension not wanting that, it should be easy to set a margin themselves.
Flags: needinfo?(mjaritz) → needinfo?(sebastian.noack)
Attached image screen.png — 
Flags: needinfo?(sebastian.noack)

    
    

    
  
I uploaded a screenshot, but it might not be too helpful, since when I use xwd, almost the whole area of the popup is blacked out, for some reason. You can closely look at the left most pixels to get an idea how it looks like though, or just try out the extension I linked above, and just remove the margin I added as workaround in popup.html.

Another Issue, that I just noticed, if I use a dark GTK theme, I get black font on almost black background, in the popup. This probably should be addressed by browser_style as well.

As for adding a margin when not using browser_style, this is what Chrome does. So doing the same would make porting existing extensions easier. Also it's not a real issue for extensions that want to use a custom margin as they can just override the user agent stylesheet, but a margin of 0 is almost always undesired IMHO, except perhaps in the scenario you mentioned.

    
    

    
  
I agree, when not using browser_style, <body> should have a default margin, as any website through user agent styling.
From a quick check, it looks like that is what we do.
As for browser_style, it is certainly something we need to refine, but I do not think it should have a default margin set, but only set what contributes to any html looking as close to our browser style as possible.
Good catch with the dark theme, that is certainly something we need to watch out for with browser style too. Thanks.

    
  
Product: Toolkit → WebExtensions

    
    

    
  
I’m pretty sure that the lack of margins is because of the Firefox Panel Components, that are part of the browser style CSS.

https://developer.mozilla.org/docs/Mozilla/Add-ons/WebExtensions/user_interface/Browser_styles#Firefox_Panel_Components

    
    

    
  
I'm an Outreachy applicant. Can I start working on this?


Flags: needinfo?(rob)

    
    

    
  
When a bug is unassigned, you're free to work on it.


This bug is however not a good first bug nor a fitting bug for Outreachy. The problem here is not "fixing" the bug (it would be an easy change in a stylesheet), but determining whether we want to change the style at all.


Flags: needinfo?(rob)

    
  
Severity: normal → S3

    
    

    
  
Closing bug because support for browser_style feature is going to be removed (at least in MV3), see bug 1827910.


Status: UNCONFIRMED → RESOLVED
Closed: 1 year ago
Resolution: --- → WONTFIX

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: