Container of browser.xul identity-popup-more-info-button (an hbox) is missing ID

RESOLVED FIXED in Firefox 4.0b9

Status

()

Firefox
General
--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Debloper, Assigned: Debloper)

Tracking

Trunk
Firefox 4.0b9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b8) Gecko/20100101 Firefox/4.0b8

In the site identity-popup, the last element ("More Information..." button, id="identity-popup-more-info-button") is contained within an hbox without any id.

If for any reason, a button is required to be added in that hbox set, it takes relatively complex approach to get it done. Adding an id to the hbox can fix it.

Even at the time of this bug-reporting, the latest revision of browser.xul has this problem - http://hg.mozilla.org/mozilla-central/file/a05e91710adb/browser/base/content/browser.xul

Reproducible: Always
(Assignee)

Comment 1

7 years ago
Created attachment 500658 [details]
Screenshot of the identity-popup
(Assignee)

Comment 2

7 years ago
Created attachment 500659 [details]
Solution code as text-file
(Assignee)

Updated

7 years ago
Version: unspecified → Trunk
Confirmed with Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20110101 Firefox/4.0b9pre ID:20110101030401
Status: UNCONFIRMED → NEW
Component: General → Theme
Ever confirmed: true
QA Contact: general → theme

Updated

7 years ago
Component: Theme → General
QA Contact: theme → general

Updated

7 years ago
Whiteboard: [good first bug]
(Assignee)

Comment 4

7 years ago
Created attachment 500696 [details] [diff] [review]
Added id="identity-popup-more-info" to the hbox to fix

Not sure if I've added the reviewer string right...
Attachment #500696 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #500696 - Flags: review? → review?(johnath)
Comment on attachment 500696 [details] [diff] [review]
Added id="identity-popup-more-info" to the hbox to fix

Please see https://developer.mozilla.org/en/Creating_a_patch and https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_diff_and_patch_files.3F
Attachment #500696 - Flags: review?(johnath)
(Assignee)

Comment 6

7 years ago
Created attachment 500752 [details] [diff] [review]
Diff created after adding id="identity-popup-more-info" to the hbox to fix

I guess, dao is available for this, & hope I've done the right thing!
Attachment #500696 - Attachment is obsolete: true
Attachment #500752 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #500752 - Flags: review? → review?(dao)
Comment on attachment 500752 [details] [diff] [review]
Diff created after adding id="identity-popup-more-info" to the hbox to fix

Your editor replaced all \n line endings with \r\n ones, resulting in an enormous patch. Please fix ;)
Attachment #500752 - Flags: review?(dao)
(Assignee)

Comment 8

7 years ago
Created attachment 500759 [details] [diff] [review]
Added id="identity-popup-more-info" to the hbox to fix

Thanks to irc:Kwierso for helping me out.
I'm pretty sure, I got it right this time.

@Dao: thanks for having patients with me; really appreciate it.
Attachment #500752 - Attachment is obsolete: true
Attachment #500759 - Flags: review?(dao)
I'd suggest "identity-popup-button-container" as the id, given this rationale:

> If for any reason, a button is required to be added in that hbox set, it takes
> relatively complex approach to get it done. Adding an id to the hbox can fix
> it.
(Assignee)

Comment 10

7 years ago
(In reply to comment #9)
> I'd suggest "identity-popup-button-container" as the id, given this rationale:
> 
> > If for any reason, a button is required to be added in that hbox set, it takes
> > relatively complex approach to get it done. Adding an id to the hbox can fix
> > it.

Nice suggestion & I've also given similar thoughts before.
One thing kept me from doing it is, all the child nodes descendant from "identity-popup" are inheriting the 1st part of their parent's id, then appending their own. But giving that ID will break it for the contained button's id. (I'd request taking a look at lines 342-384)

Although I know having "identity-popup-button-container" sounds much better, but this previous convention (albeit not mine) is not very bad anyway. I have no bias, for any id (all I want is a ID, which should be added there).

Let me know; I'll change & resubmit.
(In reply to comment #10)
> Although I know having "identity-popup-button-container" sounds much better,
> but this previous convention (albeit not mine) is not very bad anyway.

It's not an intended convention and doesn't need to be followed through.
(Assignee)

Comment 12

7 years ago
Created attachment 500878 [details] [diff] [review]
Changed ID to Dao suggested value
Attachment #500759 - Attachment is obsolete: true
Attachment #500878 - Flags: review?(dao)
Attachment #500759 - Flags: review?(dao)

Updated

7 years ago
Attachment #500878 - Flags: review?(dao) → review+

Updated

7 years ago
Attachment #500878 - Flags: approval2.0?
Attachment #500878 - Flags: approval2.0? → approval2.0+

Updated

7 years ago
Keywords: checkin-needed

Updated

7 years ago
Assignee: nobody → debloper
http://hg.mozilla.org/mozilla-central/rev/37e106e25d9c
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b9
You need to log in before you can comment on or make changes to this bug.