1139182 - Fix auto-sizing of extensions about dialog

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Sebastian Zartner [:sebo]]

Attachment: About dialog showing problem of vertical overflow

The default 'About' dialog of extensions is not properly resized on it's content if there's more than one contributor. In that case there will be a vertical scrollbar shown.

Showing that scrollbar should be avoided as far as possible. Of course there should be a maximum height to avoid occupying the whole screen when the list of contributors is very long. An example for a long list of contributors is Firebug[1]

Test case:
1. Install the Pixel Perfect 2.0b1[1]
2. Within the Add-ons Manager right-click the Pixel Perfect entry and choose 'About'

=> The shown dialog displays a scrollbar for the list of contributors as shown within the attached screenshot.

Sebastian

[1] https://getfirebug.com/releases/pixelperfect/2.0/pixelperfect-2.0.0-beta.1.xpi
[2] https://getfirebug.com/releases/firebug/2.0/firebug-2.0b8.xpi

Comment 1

[Dave Schloss]

Hi Sebastian,

I'd like to start working on this bug.  My build environment is good to go and I'll look into Firebug as an example.

-Dave

Comment 2

[Sebastian Zartner [:sebo]]

Great! I assigned the bug to you.
Note that I am only the reporter of the bug, not its mentor. So if you need help with the code, you may ask within IRC or CC someone here and ask for guidance.

Sebastian

Comment 3

[Dave Schloss]

(In reply to Sebastian Zartner [:sebo] from comment #2)
> Great! I assigned the bug to you.
> Note that I am only the reporter of the bug, not its mentor. So if you need
> help with the code, you may ask within IRC or CC someone here and ask for
> guidance.
> 
> Sebastian

Sounds good.  The bug doesn't seem too difficult, and the only real question I had was the OS environment for testing.  I was able to replicate the bug in Windows 7 as well, but I saw above that it was reported for Windows 8.1 so I'll ask in IRC if I have to verify in that OS environment before patch submission.

Comment 4

[Sebastian Zartner [:sebo]]

Dave, this bug is assigned to you for five months now. Are there any updates?

Sebastian

Comment 5

[Dave Schloss]

Life events occurred. No significant progress made.  Assigned back to Sebastian.

Comment 6

[Sebastian Zartner [:sebo]]

(In reply to Dave Schloss from comment #5)
> Life events occurred. No significant progress made.  Assigned back to
> Sebastian.

Thank you for the answer! Note that I was just the reporter, I wasn't assigned.
I currently do not plan to work on this, so I removed the assignment again to give somebody else the chance to fix this.

Sebastian

Comment 7

[eric.yan828]

Hi Sebastian,

I would like to work on this bug as my first good bug, and also as my assignment. Could you assign this bug to me?

Eric

Comment 8

[Sebastian Zartner [:sebo]]

Done. Note that for any questions on this you should ask Neil Deakin, as he's the mentor of this bug.

Sebastian

Comment 9

[eric.yan828]

(In reply to Sebastian Zartner [:sebo] from comment #8)
> Done. Note that for any questions on this you should ask Neil Deakin, as
> he's the mentor of this bug.
> 
> Sebastian

Thank you Sebastian!
I am working on it.

Eric

Comment 10

[eric.yan828]

Hi Sebastian,

It looks like this bug has been fixed. The Pixel Perfect has updated to 2.0.10. Both of the Firebug and Pixel Perfect's 'About' dialogs have reached the maximum height. Could you check it out to see if there is any problem?

Thank you.

Eric

Comment 11

[Sebastian Zartner [:sebo]]

Attachment: Test extension

I installed the beta 1 of Pixel Perfect attached to this bug on Firefox Nightly 47.0a1 (2016-02-06) and the scrollbar is still there when I open the 'About' dialog.
I've attached a test extension, so you can reproduce the issue there.

Sebastian

Comment 12

[eric.yan828]

(In reply to Sebastian Zartner [:sebo] from comment #11)
> Created attachment 8716689 [details]
> Test extension
> 
> I installed the beta 1 of Pixel Perfect attached to this bug on Firefox
> Nightly 47.0a1 (2016-02-06) and the scrollbar is still there when I open the
> 'About' dialog.
> I've attached a test extension, so you can reproduce the issue there.
> 
> Sebastian
Hi Sebastian,

Thanks a lot, I will double check my version and fix it. 

Eric

Comment 13

[Tony Mechelynck [:tonymec]]

UA:"Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0 SeaMonkey/2.44a1"
ID:20160208023510 en-US
c-c:4a60c0ffe9af0b68a82a6601c121e251863235e5
m-c:1cfe34ea394c66d7fa2c6dc366b05ab00e919113

I'm seeing this on SeaMonkey 2.44a1 on Linux (e.g. for the "Tabs Menu" extension for Firefox and SeaMonkey, which has a lot of translators; or for "Folderpane Tools" for Thunderbird and SeaMonkey, which has one "creator", one "developer", one "translator" and one "contributor").

The height of the about dialog varies from add-on to add-on even while keeping its scrollbar. :-?

The dialog is resizable (at least on Linux) but doesn't keep its user-set height after closing and reopening.

I assume that 32-bit builds are equally affected.

Comment 14

[eric.yan828]

Attachment: Test-pixelPerfectBeta1.PNG

Browser: Firefox Nightly 47.0a1 (2016-02-17)
Extension: PixelPerfect v.2.0.0-beta.1

Comment 15

[eric.yan828]

Attachment: Test-testExtension.PNG

Browser: Firefox Nightly 47.0a1 (2016-02-17)
Extension: test extension

Comment 16

[eric.yan828]

Hi Sebastian,

I attached the screen capture of the 'About' dialog with version info. Is it same as what it looks like for your? There is no dialog there, and any problem there?

Thanks,

Eric

Comment 17

[Sebastian Zartner [:sebo]]

Attachment: Windows 7 screenshot of dialog having overflow problem

> Is it same as what it looks like for your?

Obviously not. See the new attached screenshot (done in Nightly 47.0a1 2016-02-17).

So far, it's confirmed that the issue appears on
- Firefox on Windows 7, 8.1 and 10.
- SeaMonkey on Linux with X11.

Which OS are you using?

Sebastian

Comment 18

[eric.yan828]

(In reply to Sebastian Zartner [:sebo] from comment #17)
> Created attachment 8720746 [details]
> Windows 7 screenshot of dialog having overflow problem
> 
> > Is it same as what it looks like for your?
> 
> Obviously not. See the new attached screenshot (done in Nightly 47.0a1
> 2016-02-17).
> 
> So far, it's confirmed that the issue appears on
> - Firefox on Windows 7, 8.1 and 10.
> - SeaMonkey on Linux with X11.
> 
> Which OS are you using?
> 
> Sebastian

I see. I am using Linuxmint 17.3.

Eric

Comment 19

[Sebastian Zartner [:sebo]]

So, may it be possible that you try it on another system to be able to reproduce it?
Otherwise, I assume, it's hard to fix.

Sebastian

Comment 20

[Tony Mechelynck [:tonymec]]

Attachment: about dialog for Tabs Menu in seamonkey 2.44a1 (Toolkit 47.0a1) for linux-x86_64

(In reply to Tony Mechelynck [:tonymec] from comment #13)
> UA:"Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
> SeaMonkey/2.44a1"
> ID:20160208023510 en-US
> c-c:4a60c0ffe9af0b68a82a6601c121e251863235e5
> m-c:1cfe34ea394c66d7fa2c6dc366b05ab00e919113
[…]

FWIW, I'm on openSUSE Leap 42.1 and this is the only SeaMonkey 2.44a1 so far, from https://l10n.mozilla-community.org/~akalla/unofficial/seamonkey/nightly/latest-comm-central-linux64/ (built from official sources AFAIK, but not on Mozilla computers).

Comment 21

[nejmeddine.douma]

Hello,

I would like to work on this bug. I am a new here.
I already setup my build environment and I reproduced the bug under FireFox Nightly 49.0a1 (2016-05-18).
Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0

Can anyone help me from where I can start to find the files related to the bug?

Thanks

Comment 22

[Sebastian Zartner [:sebo]]

The related files are:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/about.xul
http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/mozapps/extensions/about.css

Sebastian

Comment 23

[nejmeddine.douma]

Attachment: bug-1139182-fix.patch

Comment 24

[nejmeddine.douma]

Hello,

I tried to adapt extensionDetailsBox in the extension's "about" dialog according to the number of details to be displayed. Could you please give me your feedback and assign the bug to me.

Nejmeddine

Comment 25

[Sebastian Zartner [:sebo]]

Thank you for the patch! I've assigned the bug to you.

Unfortunately I cannot test your patch, because I don't have a working build environment at the moment.
Dave, as you mentor other bugs in this component, could you please have a look here, too?

Sebastian

Comment 26

[Dave Townsend [:mossop]]

Unfortunately I don't work on this code right now. Rob, can you help here?

Comment 27

[Robert Helmer [:rhelmer]]

Sure I can take a look.

Comment 28

[Robert Helmer [:rhelmer]]

Comment on attachment 8754455 [details] [diff] [review]
bug-1139182-fix.patch

Review of attachment 8754455 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/content/about.js
@@ +72,5 @@
>      extensionDetailsBox.hidden = true;
> +  } else {
> +    if (numDetails > 5){
> +      var extensionDetailsBox = document.getElementById("extensionDetailsBox");
> +      extensionDetailsBox.style.height = "150px";

If you are changing this in `about.css`, why is it necessary here as well?

Comment 29

[nejmeddine.douma]

Thank you for the feedback. I understood the problem as: avoid showing the scrollbar as far as possible and if we show it, the number of lines hidden have to be worth scrolling to see them.I thought that if I only change the CSS min-height from 100px to 150px I will only translate the problem and we will have cases in which there will be a scrollbar only to hide half a line for example. So I tried to make sure that if there will be a scrollbar, there will be many lines hidden and at he same time, if there is not a lot of lines (less than 5) we may have a box with a height that might be slighly bigger than 150px and show all the lines without a scrollbar.
Should I send another patch with only 'about.css' edited?

Comment 30

[Robert Helmer [:rhelmer]]

(In reply to nejmeddine.douma from comment #29)
> Thank you for the feedback. I understood the problem as: avoid showing the
> scrollbar as far as possible and if we show it, the number of lines hidden
> have to be worth scrolling to see them.I thought that if I only change the
> CSS min-height from 100px to 150px I will only translate the problem and we
> will have cases in which there will be a scrollbar only to hide half a line
> for example. So I tried to make sure that if there will be a scrollbar,
> there will be many lines hidden and at he same time, if there is not a lot
> of lines (less than 5) we may have a box with a height that might be slighly
> bigger than 150px and show all the lines without a scrollbar.
> Should I send another patch with only 'about.css' edited?

Hm, I actually didn't know about the context->about modal, I actually see a lot of quirky bugs with it, just looking at my own add-ons. Just the fact that we are trying to stuff all this in a modal instead of the "Show More Information" page seems like a weird constraint.

Andy - I feel like we should remove this "About" dialog, and put all info on the "More" link in about:addons, what do you think?

Is there any reason we need both of these?

Comment 31

[Robert Helmer [:rhelmer]]

Attachment: example of the "About" modal

Comment 32

[Robert Helmer [:rhelmer]]

Attachment: example of the "Show More Information" aka "More" page

Comment 33

[Robert Helmer [:rhelmer]]

Attachment: example of "About" page with contributors

The one thing the "About" modal has is a list of contributors, here's one with a particularly large number of contributors that shows the scrollbar this bug describes.

Comment 34

[Sebastian Zartner [:sebo]]

(In reply to Robert Helmer [:rhelmer] from comment #30)
> Andy - I feel like we should remove this "About" dialog, and put all info on
> the "More" link in about:addons, what do you think?
> 
> Is there any reason we need both of these?

Good point! I'm all for it. There's already bug 571477 asking for this. If you decide so, I suggest to mark this bug as WONTFIX and focus on bug 571477 instead.

Side note: Extension authors can currently provide their own dialog via em:aboutURL, which is what I'm doing in my own add-on Regular Expressions Tester (https://addons.mozilla.org/firefox/addon/rext/). This feature should either be kept or needs to be replaced by something new allowing to replace the default.

Sebastian

Comment 35

[Andy McKay]

> Andy - I feel like we should remove this "About" dialog, and put all info on
> the "More" link in about:addons, what do you think?

I agree, I don't see any value in the "About" dialog.

In WebExtensions we have the options_ui implemented so you can add HTML to the "more" page and I would guess that is way more often visited than the "About" dialog. I don't have any stats to back this up however and currently mxr is down, when it's back I'm curious to see how many add-ons implement aboutURL. I bet its hardly any.

Comment 36

[Sebastian Zartner [:sebo]]

(In reply to Andy McKay [:andym] from comment #35)
> > Andy - I feel like we should remove this "About" dialog, and put all info on
> > the "More" link in about:addons, what do you think?
> 
> I agree, I don't see any value in the "About" dialog.

Ok, so I close this then. Sorry Nejmeddine for the time you've spend on this! Maybe you could have a look at bug 571477 instead (which I assume is a bit harder to implement, though)?

> In WebExtensions we have the options_ui implemented so you can add HTML to
> the "more" page and I would guess that is way more often visited than the
> "About" dialog. I don't have any stats to back this up however and currently
> mxr is down, when it's back I'm curious to see how many add-ons implement
> aboutURL. I bet its hardly any.

Probably, yes. It would be sad to lose extensibility, though, if your intention behind this statement is to remove the ability to customize it. But let's continue this discussion in bug 571477.

Sebastian

Comment 37

[Sebastian Zartner [:sebo]]

(In reply to Andy McKay [:andym] from comment #35)
> In WebExtensions we have the options_ui implemented so you can add HTML to
> the "more" page and I would guess that is way more often visited than the
> "About" dialog. I don't have any stats to back this up however and currently
> mxr is down, when it's back I'm curious to see how many add-ons implement
> aboutURL. I bet its hardly any.

https://dxr.mozilla.org/addons/search?q=em%3AaboutURL&redirect=false returns 4,783 matches, which looks like a pretty large number.

Sebastian

Comment 38

[Andy McKay]

(In reply to Sebastian Zartner [:sebo] from comment #37)
> https://dxr.mozilla.org/addons/search?q=em%3AaboutURL&redirect=false returns
> 4,783 matches, which looks like a pretty large number.

Golly I'm surprised, that's a lot more than I would have thought. Still I stand by the fact that we have lots of other places to put this information already and maintaining this doesn't seem worth it to anyone.

Comment 39

[Sebastian Zartner [:sebo]]

(In reply to Andy McKay [:andym] from comment #38)
> (In reply to Sebastian Zartner [:sebo] from comment #37)
> > https://dxr.mozilla.org/addons/search?q=em%3AaboutURL&redirect=false returns
> > 4,783 matches, which looks like a pretty large number.
> 
> Golly I'm surprised, that's a lot more than I would have thought. Still I
> stand by the fact that we have lots of other places to put this information
> already and maintaining this doesn't seem worth it to anyone.

Yes, as mentioned before, this should be discussed in bug 571477. I've added the info there, too, now.

Sebastian