Last Comment Bug 714342 - page-proxy-stack should be a <hbox> rather than a <stack>
: page-proxy-stack should be a <hbox> rather than a <stack>
Status: RESOLVED FIXED
[good first bug][mentor=dao][lang=xul]
:
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: Firefox 12
Assigned To: Murali
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-30 12:20 PST by Dão Gottwald [:dao]
Modified: 2012-02-01 13:57 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
page-proxy-stack made into a <hbox> element (1.54 KB, patch)
2012-01-23 02:19 PST, Murali
no flags Details | Diff | Splinter Review
page-proxy-stack made into a <hbox> element (alignment fixed) (1.54 KB, patch)
2012-01-23 02:33 PST, Murali
no flags Details | Diff | Splinter Review
page-proxy-stack made into a <hbox> element (1.60 KB, patch)
2012-01-23 02:39 PST, Murali
dao+bmo: review+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2011-12-30 12:20:39 PST
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.xul#539

page-proxy-stack contains only one element (page-proxy-favicon), there's nothing to be stacked. We should use a plain box element for this, not a stack element. We should however keep the "page-proxy-stack" id for backwards-compatibility with add-ons.
Comment 1 Murali 2012-01-22 22:19:02 PST
Hi, I would like to work on this bug. Can you please mentor me? This is my first bug. Thanks a lot!
Comment 2 Dão Gottwald [:dao] 2012-01-23 01:13:20 PST
Hi! Do you already have a copy of the source code? https://developer.mozilla.org/en/Mozilla_Source_Code_%28Mercurial%29 has some information on this.
Comment 3 Murali 2012-01-23 01:25:25 PST
I already got the source code from https://developer.mozilla.org/En/Simple_Firefox_build . Could you please tell me whether it is the same? Or should I follow the instructions from the link you provided? Thanks!
Comment 4 Dão Gottwald [:dao] 2012-01-23 01:44:30 PST
It's the same. You can now open browser/base/content/browser.xul in your editor, find the line containing '<stack id="page-proxy-stack"' and make this a "hbox" element instead of a "stack" element.
Comment 5 Murali 2012-01-23 02:19:01 PST
Created attachment 590664 [details] [diff] [review]
page-proxy-stack made into a <hbox> element
Comment 6 Dão Gottwald [:dao] 2012-01-23 02:23:56 PST
Comment on attachment 590664 [details] [diff] [review]
page-proxy-stack made into a <hbox> element

>+              <hbox id="page-proxy-stack"
>                      onclick="PageProxyClickHandler(event);">

nit-pick: please adjust the second line's indentation so that the attributes line up again
Comment 7 Murali 2012-01-23 02:33:28 PST
Created attachment 590665 [details] [diff] [review]
page-proxy-stack made into a <hbox> element (alignment fixed)

Alignment fixed...?
Comment 8 Dão Gottwald [:dao] 2012-01-23 02:35:12 PST
Comment on attachment 590665 [details] [diff] [review]
page-proxy-stack made into a <hbox> element (alignment fixed)

You seem to have modified the patch. Instead, you need to modify the source and generate a new patch.
Comment 9 Murali 2012-01-23 02:39:36 PST
Created attachment 590666 [details] [diff] [review]
page-proxy-stack made into a <hbox> element

So sorry, I did not know it made a difference.
Comment 10 Dão Gottwald [:dao] 2012-01-23 02:59:18 PST
Comment on attachment 590666 [details] [diff] [review]
page-proxy-stack made into a <hbox> element

Thanks!
Comment 11 Murali 2012-01-23 03:00:34 PST
Thank you so much for guiding me!! :)
Comment 13 Ed Morley [:emorley] 2012-01-23 11:49:02 PST
https://hg.mozilla.org/mozilla-central/rev/79cb2db62a9b

Thank you for the patch! This will be in tomorrow's nightly. If you need any inspiration for things to fix next, pop on #developers @ irc.mozilla.org :-)

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