Remove the 'viewbutton' XBL binding and pageInfo.xml

RESOLVED FIXED in Firefox 58

Status

()

Firefox
Page Info Window
RESOLVED FIXED
a month ago
13 days ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

(Blocks: 3 bugs)

unspecified
Firefox 58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

(Whiteboard: [xbl-flatten-inheritance])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

a month ago
The viewbutton binding seems to be a way to control the display of the radiogroup radio headers in the page info window (i.e. "General", "Media", "Security", etc).

We used to support extensions injecting new headers through an overlay (https://dxr.mozilla.org/mozilla-central/source/browser/base/content/pageinfo/pageInfo.xul#84). Since that's not needed anymore, I'm assuming we can remove the XBL binding and modify the markup as needed in the radiogroup to keep things looking the same.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

28 days ago
mozreview-review
Comment on attachment 8920708 [details]
Bug 1410540 - Remove the 'viewbutton' XBL binding and pageInfo.xml;

https://reviewboard.mozilla.org/r/191706/#review198060

::: commit-message-d1e99:4
(Diff revision 3)
> +Bug 1410540 - Remove the 'viewbutton' XBL binding and pageInfo.xml;r=dao
> +
> +Instead of creating a new binding that extends the radio binding to remove the actual
> +radio input from the DOM, with this change we use the platform-specific radio bindings

Hmm, we should get rid of the platform-specific bindings, which should simplify this patch. I did that for <checkbox/> in bug 1345432. Mind doing the same here?
Attachment #8920708 - Flags: review?(dao+bmo)
(Assignee)

Updated

28 days ago
Depends on: 1411640
(Assignee)

Comment 7

28 days ago
(In reply to Dão Gottwald [::dao] from comment #6)
> Comment on attachment 8920708 [details]
> Bug 1410540 - Remove the 'viewbutton' XBL binding and pageInfo.xml;
> 
> https://reviewboard.mozilla.org/r/191706/#review198060
> 
> ::: commit-message-d1e99:4
> (Diff revision 3)
> > +Bug 1410540 - Remove the 'viewbutton' XBL binding and pageInfo.xml;r=dao
> > +
> > +Instead of creating a new binding that extends the radio binding to remove the actual
> > +radio input from the DOM, with this change we use the platform-specific radio bindings
> 
> Hmm, we should get rid of the platform-specific bindings, which should
> simplify this patch. I did that for <checkbox/> in bug 1345432. Mind doing
> the same here?

That sounds like a good idea and should ultimately make migration to a Custom Element for radios easier - I filed Bug 1411640 for that
(Assignee)

Updated

26 days ago
Blocks: 1412369

Comment 8

21 days ago
mozreview-review
Comment on attachment 8921182 [details]
Bug 1410540 - Merge viewbutton.css into pageInfo.css for osx;

https://reviewboard.mozilla.org/r/192170/#review200412

not sure if this patch is still relevant on its own
Attachment #8921182 - Flags: review?(dao+bmo)
(Assignee)

Comment 9

21 days ago
(In reply to Dão Gottwald [::dao] from comment #8)
> not sure if this patch is still relevant on its own

It's not - this is blocked on Bug 1411640
(Assignee)

Updated

20 days ago
Whiteboard: [xbl-flatten-inheritance]
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 12

15 days ago
Rebased and updated now that we have a unified radio binding after bug 1411640
(Assignee)

Updated

15 days ago
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED

Comment 13

14 days ago
mozreview-review
Comment on attachment 8920708 [details]
Bug 1410540 - Remove the 'viewbutton' XBL binding and pageInfo.xml;

https://reviewboard.mozilla.org/r/191706/#review202848

::: commit-message-d1e99:4
(Diff revision 4)
> +Bug 1410540 - Remove the 'viewbutton' XBL binding and pageInfo.xml;r=dao
> +
> +Instead of creating a new binding that extends the radio binding to remove the actual
> +radio input from the DOM, with this change we use the platform-specific radio bindings

s/platform-specific radio bindings/default radio binding/
Attachment #8920708 - Flags: review?(dao+bmo) → review+

Comment 14

14 days ago
mozreview-review
Comment on attachment 8921182 [details]
Bug 1410540 - Merge viewbutton.css into pageInfo.css for osx;

https://reviewboard.mozilla.org/r/192170/#review202850
Attachment #8921182 - Flags: review?(dao+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 17

14 days ago
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/765f81d5cd73
Remove the 'viewbutton' XBL binding and pageInfo.xml;r=dao
https://hg.mozilla.org/integration/autoland/rev/f1b40946cdbe
Merge viewbutton.css into pageInfo.css for osx;r=dao
https://hg.mozilla.org/mozilla-central/rev/765f81d5cd73
https://hg.mozilla.org/mozilla-central/rev/f1b40946cdbe
Status: ASSIGNED → RESOLVED
Last Resolved: 13 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.