Last Comment Bug 640677 - Separator is double in HELP menu of "View Source" and "View selected Source" window
: Separator is double in HELP menu of "View Source" and "View selected Source" ...
Status: RESOLVED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: Trunk
: All All
: -- trivial (vote)
: Firefox 10
Assigned To: Vlad Tanase
:
: Jared Wein [:jaws] (please needinfo? me)
Mentors:
Depends on:
Blocks: 542122
  Show dependency treegraph
 
Reported: 2011-03-10 10:38 PST by Ilja Nedilko
Modified: 2011-11-07 13:21 PST (History)
7 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot (23.42 KB, image/png)
2011-03-10 11:14 PST, Alice0775 White
no flags Details
Patch to remove the double separator in the help menu of the view source page. (955 bytes, patch)
2011-10-09 07:18 PDT, Vlad Tanase
no flags Details | Diff | Splinter Review
Patch to remove the double separator in the help menu of the view source page. (1.02 KB, patch)
2011-10-09 08:48 PDT, Vlad Tanase
dolske: review+
Details | Diff | Splinter Review

Description Ilja Nedilko 2011-03-10 10:38:44 PST
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0

There are two horizontal lines in between "Restart with add-ons disabled..." and "About firefox", in "View page source" (ctrl + u).

Reproducible: Always

Steps to Reproduce:
1. Go to "Firefox", "Web developer", "View page source".
2. Go to "Help menu".
3. There are two horizontal lines in between "Restart with add-ons disabled..." and "About firefox".
Actual Results:  
Two horizontal lines appear.

Expected Results:  
I assume that just one horizontal line needs to be in between those two fields.

Screenshot: http://postimage.org/image/1dzz1em4k/
Comment 1 Ilja Nedilko 2011-03-10 10:42:01 PST
Sorry, made a tiny mistake in the title, it's not located in "View" menu but in "Help".
Comment 2 Alice0775 White 2011-03-10 11:14:00 PST
Created attachment 518452 [details]
Screenshot

Separator is double in HELP menu of "View Source" and "View selected Source" window
Comment 3 [not reading bugmail] 2011-03-10 11:41:57 PST
That's weird, my build is fine, as it says Report Web Forgery inbetween the separators.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110310 Firefox/4.0b13pre ID:20110310030427
Comment 4 [not reading bugmail] 2011-03-10 11:43:31 PST
(In reply to comment #3)
> That's weird, my build is fine, as it says Report Web Forgery inbetween the
> separators.
> 
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110310
> Firefox/4.0b13pre ID:20110310030427

That is different then the view source screen.  Confirming the view source screen is different then the main window help menu.
Comment 5 Vlad Tanase 2011-10-09 07:18:52 PDT
Created attachment 565804 [details] [diff] [review]
Patch to remove the double separator in the help menu of the view source page.

Hello,

I added a patch to correct this issue. This is my first time submitting code to firefox so I hope everything is according to the expected work flow. If there are any problems with the submitted modifications I will do my best to fix them in a timely manner.

Vlad
Comment 6 Vlad Tanase 2011-10-09 08:48:24 PDT
Created attachment 565808 [details] [diff] [review]
Patch to remove the double separator in the help menu of the view source page.

Updated the patch according to this page: https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Vlad
Comment 7 Justin Dolske [:Dolske] 2011-10-09 16:28:14 PDT
Comment on attachment 565808 [details] [diff] [review]
Patch to remove the double separator in the help menu of the view source page.

Hmm, the extra separator was added in bug 542122 (which was adding the "Restart With Add-ons Disabled…" menuitem.

The XUL looks kind of dumb at first glance, but that's because report-phishing-overlay.xul has |insertbefore="aboutSeparator"|, which addds the "Report Web Forgery…" menuitem between the two separators. The double-separator bug manifests whenever this overlay isn't included.

I suppose we could aim for a more complicated patch that preserves the current visual appearance by only sometimes removing the extra separator... But I think menu's already a bit of a mess, and the separators to put Report Web Forgery in its own block doesn't seem useful.

So let's just go with your simple patch. :)

Thanks for the patch!
Comment 8 Ed Morley [:emorley] 2011-10-09 23:46:08 PDT
Thanks for the patch! 

It has landed in mozilla-inbound and will get merged to mozilla-central in <24 hours (https://wiki.mozilla.org/Tree_Rules/Inbound#What_is_mozilla-inbound.3F), where you will then see it in the ~11th+ Oct Nightly / will be visible to end-users in Firefox 10.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d61f3ed73b9

Hopefully see you on IRC (http://irc.mozilla.org/) in #developers (my nick is edmorley) - if you'd like to fix another bug (we'd love it if you did!) but need some inspiration, pop on and we'll find something for you :-)
Comment 9 Matt Brubeck (:mbrubeck) 2011-10-10 11:12:10 PDT
https://hg.mozilla.org/mozilla-central/rev/4d61f3ed73b9
Comment 10 Vlad Tanase 2011-10-10 11:28:12 PDT
Wow this went faster than expected.

Thank you for being so responsive guys. Will definitely be dropping by on IRC and fixing some more bugs should I have the time. :-)
Comment 11 [:Aleksej] 2011-11-07 13:15:15 PST
FWIW, this is present in the Aurora nightly and Firefox 8.
Comment 12 Ed Morley [:emorley] 2011-11-07 13:21:00 PST
(In reply to Aleksej [:Aleksej] from comment #11)
> FWIW, this is present in the Aurora nightly and Firefox 8.

Thanks for the confirmation, though this is expected. Comment 7 mentions that this issue was introduced by bug 542122, which landed for Firefox 4b7 - so the problem has existed since then. The fix here has landed for Firefox 10 (see target milestone field at the top of this page), so won't be present in the Aurora (9) or Firefox 8 builds you tested.

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