725252 - "Bookmark All Tabs" dialog window doesn't resize properly (gtk)

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Patrick]

Attachment: Screenshot at 2012-02-08 11:51:41.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0
Build ID: 20120129141257

Steps to reproduce:

- Right click on any Tab in the tab bar
- choose "Bookmark All Tabs"
- resize (enlarge) the window vertically


Actual results:

The window resized but the elements within did not


Expected results:

The elements within (bookmarks folder tree) should have been resized to fill the available space inside the enlarged window.

Comment 1

[Tim (fmdeveloper)]

Marco -> Is this a bookmark issue or something else?

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

yes, it's not a gtk/widget issue it's just that nothing inside the dialog is vertically resizable.

Comment 3

[Seif Lotfy]

Attachment: 0001-Bug-725252-Bookmark-All-Tabs-dialog-window-doesn-t-r.patch

There were missing flex properties in the xul. I added those + a minheight.

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 8351438 [details] [diff] [review]
0001-Bug-725252-Bookmark-All-Tabs-dialog-window-doesn-t-r.patch

No need to tag me here; I don't know this code at all. No superreview required either.

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8351438 [details] [diff] [review]
0001-Bug-725252-Bookmark-All-Tabs-dialog-window-doesn-t-r.patch

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

I was a bit worried cause I was faintly remembering there was a reason we didn't make this and moreover the overlay is spread into many dialogs. But I tested it and it doesn't seem to cause any issue. I suspect some of the changes we made to the properties dialog last year may have solved some problem.
The patch is not solving all of the issues with this resizable dialog (if you don't expand the folder picker and resize the window, you still get a large grey area, while the picker should probably expand when over a certain threshold), but it's exactly solving this bug as it is filed, so I'd say we can take it and see, other issues may be handled apart in separate reports.

Before setting the checkin-needed keyword, please fix the commit message, so that it's in the usual form "Bug ###### - Description of the change. r=reviewer".

Thank you

Comment 6

[Seif Lotfy]

Attachment: 0001-Bug-725252-Bookmark-All-Tabs-dialog-window-doesn-t-r.patch

Voila :D

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a8906b3529a3

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8358785 [details] [diff] [review]
0001-Bug-725252-Bookmark-All-Tabs-dialog-window-doesn-t-r.patch

In the future, please just set the checkin-needed bug keyword. Thanks for the patch!

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a8906b3529a3

Comment 10

[Francesca Ciceri [:madamezou]]

[bugday-20140402]

Hi, 

I cannot reproduce the problem anymore in  Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140331125246
https://hg.mozilla.org/releases/mozilla-beta/rev/6ca67f136635

Comment 11

[Francesca Ciceri [:madamezou]]

Hi,

as target-milestone is set to 29, I'm changing the global status to Verified as per comment 10. Feel free to revert if this is not correct. 

Cheers,
Francesca