Closed Bug 1425356 Opened 6 years ago Closed 6 years ago

[meta] Remove XUL templates

Categories

(Core :: XUL, enhancement)

Product:
Core
Component:
XUL
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, meta)

Attachments

(5 files)

Bug 1425356 - remove local-build-only 'buster' XSLT qa tool,
59 bytes, text/x-review-board-request
Pike
: review+
florian
: review+
Details
Bug 1425356 - remove some tests that only make sense when XUL templates are supported,
59 bytes, text/x-review-board-request
mrbkap
: review+
Details
Bug 1425356 - stop testing treeview RDF-template persistence because we don't use it anywhere,
59 bytes, text/x-review-board-request
mrbkap
: review+
Details
Bug 1425356 - remove XUL template support,
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
Bug 1425356 - remove various dead XUL sort attributes,
59 bytes, text/x-review-board-request
florian
: review+
bzbarsky
: review+
Details 
Per discussions, we don't think we need XUL templating in Firefox.

The only usecase I can find is actually the mac window menu, which uses a ton of RDF-ridden code to do something that should be possible with about 15 lines of JS.

I'm probably wrong.

But even if I am, there's clearly not a lot of stuff here so we should just give it a shot.
Depends on: 1425363
I see 4 uses in Thunderbird and a bunch in Seamonkey.  We should at least file bugs on them to come up with an alternative.... better yet if we can suggest one.
Blocks: 1425962
Blocks: 1425963
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #1)
> I see 4 uses in Thunderbird and a bunch in Seamonkey.  We should at least
> file bugs on them to come up with an alternative.... better yet if we can
> suggest one.

Filed bug 1425962 and bug 1425963 for this.
Depends on: 1426098
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8938020 [details]
Bug 1425356 - remove various dead XUL sort attributes,

https://reviewboard.mozilla.org/r/208754/#review214472

r=me for the browser/ part, but please ask a dom reviewer to look at the dom/ changes.
Attachment #8938020 - Flags: review?(florian) → review+
Attachment #8938020 - Flags: review?(bzbarsky)
Comment on attachment 8938016 [details]
Bug 1425356 - remove local-build-only 'buster' XSLT qa tool,

https://reviewboard.mozilla.org/r/208746/#review214480

The browser_all_files_referenced.js change looks good. It seems we can now remove that old communicator.css file, but that's unrelated to what you are doing here, so it's for a follow-up :-).
Attachment #8938016 - Flags: review?(florian) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=188dacce1eb8

First push here was broken because I missed a reference in a layout/ moz.build file. Dunno why it builds fine locally and fails on try, but hey-ho.
Blocks: 1426426
Comment on attachment 8938019 [details]
Bug 1425356 - remove XUL template support,

https://reviewboard.mozilla.org/r/208752/#review214566

So much code removal.  :)

Is this going to break tbird nightlies seriously enough that we should give them a bit of time to deal before we land this?

Was the "ref" attribute on XUL elements only used for this template stuff?  If so, can we remove XULDocument::mRefMap and its corresponding complexity as a followup (and devirtualize nsIDocument::GetElementByID!)?  It looks to me per https://searchfox.org/mozilla-central/search?q=%20ref%3D%22&case=false&regexp=false&path=xul like the "ref" attribute is only used for template bits...

::: dom/xul/nsXULElement.h:333
(Diff revision 2)
>  
>  #define XUL_ELEMENT_FLAG_BIT(n_) NODE_FLAG_BIT(ELEMENT_TYPE_SPECIFIC_BITS_OFFSET + (n_))
>  
>  // XUL element specific bits
>  enum {
> -  XUL_ELEMENT_TEMPLATE_GENERATED =        XUL_ELEMENT_FLAG_BIT(0),
> +  XUL_ELEMENT_UNUSED =                    XUL_ELEMENT_FLAG_BIT(0),

It would be better to just drop this bit and renumber the other two bits.  Worth checking whether this effectively frees up a node bit, later on!

::: dom/xul/nsXULElement.h:338
(Diff revision 2)
> -  XUL_ELEMENT_TEMPLATE_GENERATED =        XUL_ELEMENT_FLAG_BIT(0),
> +  XUL_ELEMENT_UNUSED =                    XUL_ELEMENT_FLAG_BIT(0),
>    XUL_ELEMENT_HAS_CONTENTMENU_LISTENER =  XUL_ELEMENT_FLAG_BIT(1),
>    XUL_ELEMENT_HAS_POPUP_LISTENER =        XUL_ELEMENT_FLAG_BIT(2)
>  };
>  
>  ASSERT_NODE_FLAGS_SPACE(ELEMENT_TYPE_SPECIFIC_BITS_OFFSET + 3);

And this would become "+ 2" instead of "+ 3".
Attachment #8938019 - Flags: review?(bzbarsky) → review+
Comment on attachment 8938020 [details]
Bug 1425356 - remove various dead XUL sort attributes,

https://reviewboard.mozilla.org/r/208754/#review214570
Attachment #8938020 - Flags: review?(bzbarsky) → review+
Blocks: 1426525
Apologies for the many updates here. Third time lucky on the trypush, I hope...

Note that I've now updated test_interfaces.html and the package manifests (oops - I grepped those previously but in hindsight didn't use the right search string - also, seems android is fine with missing installer files, but not desktop...), and fixed the review nits from comment #17.
Comment on attachment 8938016 [details]
Bug 1425356 - remove local-build-only 'buster' XSLT qa tool,

https://reviewboard.mozilla.org/r/208746/#review214784
Attachment #8938016 - Flags: review?(l10n) → review+
Comment on attachment 8938017 [details]
Bug 1425356 - remove some tests that only make sense when XUL templates are supported,

https://reviewboard.mozilla.org/r/208748/#review215512
Attachment #8938017 - Flags: review?(mrbkap) → review+
Comment on attachment 8938018 [details]
Bug 1425356 - stop testing treeview RDF-template persistence because we don't use it anywhere,

https://reviewboard.mozilla.org/r/208750/#review215514
Attachment #8938018 - Flags: review?(mrbkap) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0c89956f724e
remove local-build-only 'buster' XSLT qa tool, r=florian,Pike
https://hg.mozilla.org/integration/autoland/rev/4b4a36a57d94
remove some tests that only make sense when XUL templates are supported, r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/490e100ac2e5
stop testing treeview RDF-template persistence because we don't use it anywhere, r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/ca9153588760
remove XUL template support, r=bz
https://hg.mozilla.org/integration/autoland/rev/886d3489da20
remove various dead XUL sort attributes, r=bz,florian
Blocks: 833098
Blocks: 732106
Blocks: 1467238
No longer blocks: 1467238
You need to log in before you can comment on or make changes to this bug.