Fix auto-sizing of extensions about dialog

RESOLVED WONTFIX

Status

()

defect
--
minor
RESOLVED WONTFIX
4 years ago
3 years ago

People

(Reporter: sebo, Assigned: nejmeddine.douma, Mentored)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox47 affected)

Details

(Whiteboard: [good first bug])

Attachments

(10 attachments)

Reporter

Description

4 years ago
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

4 years ago
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
Reporter

Comment 2

4 years ago
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
Assignee: nobody → drschloss

Comment 3

4 years ago
(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.

Updated

4 years ago
Status: NEW → ASSIGNED

Updated

4 years ago
Mentor: enndeakin
Reporter

Comment 4

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

Sebastian
Flags: needinfo?(drschloss)

Comment 5

4 years ago
Life events occurred. No significant progress made.  Assigned back to Sebastian.
Assignee: drschloss → sebastianzartner
Flags: needinfo?(drschloss)
Reporter

Comment 6

4 years ago
(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
Assignee: sebastianzartner → nobody
Status: ASSIGNED → NEW

Comment 7

3 years ago
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
Reporter

Comment 8

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

Sebastian
Assignee: nobody → eric.yan828

Comment 9

3 years ago
(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

3 years ago
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
Reporter

Comment 11

3 years ago
Posted file 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

3 years ago
(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
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.
QA Whiteboard: [seamonkey-2.44-affected]
OS: Windows 8.1 → All
Hardware: x86_64 → All

Comment 14

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

Comment 15

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

Comment 16

3 years ago
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
Reporter

Comment 17

3 years ago
> 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

3 years ago
(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
Reporter

Comment 19

3 years ago
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
(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).

Updated

3 years ago
Assignee: eric.yan828 → nobody
Assignee

Comment 21

3 years ago
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
Flags: needinfo?(enndeakin)
Assignee

Comment 23

3 years ago
Assignee

Comment 24

3 years ago
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
Reporter

Comment 25

3 years ago
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
Assignee: nobody → nejmeddine.douma
Status: NEW → ASSIGNED
Flags: needinfo?(dtownsend)
Unfortunately I don't work on this code right now. Rob, can you help here?
Flags: needinfo?(dtownsend) → needinfo?(rhelmer)
Sure I can take a look.
Flags: needinfo?(rhelmer)
Attachment #8754455 - Flags: review?(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?
Attachment #8754455 - Flags: review?(rhelmer)
Assignee

Comment 29

3 years ago
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?
Flags: needinfo?(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?
Flags: needinfo?(rhelmer) → needinfo?(amckay)
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.
Reporter

Comment 34

3 years ago
(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
See Also: → 571477

Comment 35

3 years ago
> 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.
Flags: needinfo?(amckay)
Reporter

Comment 36

3 years ago
(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
Reporter

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Reporter

Comment 37

3 years ago
(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

3 years ago
(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.
Reporter

Comment 39

3 years ago
(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
You need to log in before you can comment on or make changes to this bug.