Closed Bug 787650 Opened 8 years ago Closed 8 years ago

Add element ids to pageInfo.xul

Categories

(Firefox :: Page Info Window, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 18

People

(Reporter: atte, Assigned: atte)

Details

Attachments

(1 file)

Add element ids to pageInfo.xul to make it easier for extensions to overlay it.
Attached patch patchSplinter Review
Attachment #657551 - Flags: review?(adw)
Assignee: nobody → atte.kemppila
Severity: normal → enhancement
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 657551 [details] [diff] [review]
patch

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

::: browser/base/content/pageinfo/pageInfo.xul
@@ +273,2 @@
>          <button label="&mediaSaveAs;" accesskey="&mediaSaveAs2.accesskey;"
> +                icon="save" id="mediasaveasbutton"

It looks like you're choosing IDs that blend in with neighboring IDs, which is fine, so did you mean to use camel case here?
Attachment #657551 - Flags: review?(adw) → review+
(In reply to Drew Willcoxon :adw from comment #2)
> ::: browser/base/content/pageinfo/pageInfo.xul
> @@ +273,2 @@
> >          <button label="&mediaSaveAs;" accesskey="&mediaSaveAs2.accesskey;"
> > +                icon="save" id="mediasaveasbutton"
> 
> It looks like you're choosing IDs that blend in with neighboring IDs, which
> is fine,... 

Yes, that was my idea.

> ...so did you mean to use camel case here?

No. All lower case is intentional. My logic was that "mediasaveasbutton"'s neighbor is the existing "imagesaveasbutton". They both perform a similar function and are located at the same place in the ui. Which of them is visible depends on the selection.

(Sure, both of them are badly named at this point, but that can't be really helped.)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c87c20436def
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
You need to log in before you can comment on or make changes to this bug.