1513337 - Inline quantumbar markup into browser.xhtml

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P1)

Description

[Tim Nguyen :ntim]

The urlbar binding is markup only, so that could be inlined in browser.xul.

This would require figuring out a way to get rid of the textbox binding dependency.

Comment 1

[Tim Nguyen :ntim]

Note that the binding has some useless inherits:
- inherits="pageproxystate" on urlbar-go-button
- very likely one of the inherits on urlbar-input
- parentfocused on urlbar-history-dropmarker seems unused ?

Either way, most of the inherits usage just ease up the styling and can usually be reproduced with descendant selectors in the CSS.

Comment 3

[alexander :surkov (:asurkov)]

Dao, could you answer bug 1534331 comment #3?

Comment 4

[Dão Gottwald [:dao]]

We shouldn't use a custom element here, so the question doesn't really apply.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to Dão Gottwald [::dao] from comment #4)

We shouldn't use a custom element here, so the question doesn't really apply.

k

(In reply to Tim Nguyen :ntim from comment #0)

The urlbar binding is markup only, so that could be inlined in browser.xul.

This would require figuring out a way to get rid of the textbox binding
dependency.

so the idea is to pour urlbar content right into <textbox> children?

Comment 6

[Dão Gottwald [:dao]]

The idea is to not use a textbox at all.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Dão Gottwald [::dao] from comment #6)

The idea is to not use a textbox at all.

Are any of these properties are needed for urlbar implementation? https://searchfox.org/mozilla-central/source/toolkit/content/widgets/textbox.xml#50

So the idea is to replace textbox on ordinal container like hbox and then style it properly? How would it live together with no quantumbar implementation?

Comment 8

[Tim Nguyen :ntim]

(In reply to alexander :surkov (:asurkov) from comment #7)

(In reply to Dão Gottwald [::dao] from comment #6)

The idea is to not use a textbox at all.

Are any of these properties are needed for urlbar implementation? https://searchfox.org/mozilla-central/source/toolkit/content/widgets/textbox.xml#50

Maybe, but I think it'll probably be caught by tests if something breaks.

So the idea is to replace textbox on ordinal container like hbox and then style it properly? How would it live together with no quantumbar implementation?

Since the markup is the same for both quantumbar and non-quantumbar, I guess the markup for both urlbar bindings can be inlined, with the legacy-urlbar binding kept to attach different behaviours to the element.

Comment 9

[Tim Nguyen :ntim]

I think this bug would be easier to fix with bug 1515589 fixed first.

Comment 10

[Tim Nguyen :ntim]

Attachment: Bug 1513337 - Inline urlbar markup into browser.xhtml.

Comment 11

[Tim Nguyen :ntim]

I won't have time to finish this, but here's a possible plan if you'd like to work on this:

• Fix bug 1515589
• Remove or port the inherits="" attributes
• Fix the "X is undefined" errors
• Potentially more stuff

Comment 12

[Tim Nguyen :ntim]

Hi Dão,

Do you have some cycles to take this over the finish line? The current patch mostly works for both the old and new URL bar, but has some bugs there and there.

Some notable issues are: the go button doesn’t appear, live switching implementations doesn’t work super well.

I think it might be faster if you or someone from your team finishes this, given that you’re more familiar with this code.

Thanks.

Comment 13

[Dão Gottwald [:dao]]

We shouldn't spend time on this until after quantumbar is released and the old urlbar is removed.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to Dão Gottwald [::dao] from comment #13)

We shouldn't spend time on this until after quantumbar is released and the old urlbar is removed.

Do you mean that you/your team doesn't have cycles to finish this before quantumbar released or this work may interfere with some other planned work and thus it's better to delay it until it done? It'd be just cool to have it resolved asap for deXBLization project.

Comment 15

[Dão Gottwald [:dao]]

It may interfere and imply noise we don't need right now, but that aside what I'm saying is it makes much more sense to do this when we don't have to worry about the old urlbar anymore, which will be soon enough. The old urlbar is all XBL so that's a blocker for deXBL anyway.

Comment 16

[alexander :surkov (:asurkov)]

for the record, this bug can help fix bug 1534661, which doesn't work because aria-owns inside urlbar binding refers outside the binding, because it's not allowed. This bug suggests to pour the urlbar content into a document, what will fixe aria-owns issue.

Comment 17

[Dão Gottwald [:dao]]

ntim, this will become a priority next week once bug 1551232 has landed. Do you intend to finish this (I see you already updated the patch a few days ago) or should somebody else take this over?

Comment 18

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [::dao] from comment #17)

ntim, this will become a priority next week once bug 1551232 has landed. Do you intend to finish this (I see you already updated the patch a few days ago) or should somebody else take this over?

I don't plan on finishing this anytime soon. Feel free to take this over.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7c14ebfcd4d
Inline urlbar markup into browser.xhtml. r=mak

Comment 20

[Cristian Brindusan [:cbrindusan]]

Backed out changeset a7c14ebfcd4d (Bug 1513337) for telemetry failures at test_event_ping.py.

Backout: https://hg.mozilla.org/integration/autoland/rev/985af677760ba8325918b4e1e23d0587e04a6139

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a7c14ebfcd4dcfdba79022b538c6e59c305781fa&selectedJob=256740001

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256740001&repo=autoland&lineNumber=1352

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf34b2e8ac53
Inline urlbar markup into browser.xhtml. r=mak

Comment 22

[Cristina Coroiu [:ccoroiu]]

Backed out changeset cf34b2e8ac53 (bug 1513337) for test client failure

Backout: https://hg.mozilla.org/integration/autoland/rev/ccccfc40c7e78b7d01ea8d59f5a2e84617f567f9

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=cf34b2e8ac53762fd91c0e608fce8511e5d40091

Failure logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256881008&repo=autoland&lineNumber=1267

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256886042&repo=autoland&lineNumber=3068

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3504c0b3f87c
Inline urlbar markup into browser.xhtml. r=mak

Comment 24

[Alexandru Michis [:malexandru]]

Backed out changeset 3504c0b3f87c for marionette failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/d3e52073319254d1a2e669aab8d75fb24331075b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&tochange=d3e52073319254d1a2e669aab8d75fb24331075b&fromchange=3504c0b3f87c4ea941757a37afe5eb81f0082d88&searchStr=marionette&selectedJob=256900816

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256900816&repo=autoland&lineNumber=6819

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0c57ddf0725
Inline urlbar markup into browser.xhtml. r=mak

Comment 26

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a0c57ddf0725