1491252 - Port URL overflow handling from urlbarBindings.xml to UrlbarInput.jsm

Status: RESOLVED FIXED

(Firefox :: Address Bar, enhancement, P1)

Description

[Dão Gottwald [:dao]]

See updateTextOverflow in urlbarBindings.xml.

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch

Comment 2

[Dão Gottwald [:dao]]

Attachment: patch

rebased

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 9010596 [details] [diff] [review]
patch

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

Looks good.

For future reference, this functionality is already covered by browser/base/content/test/urlbar/browser_urlOverflow.js which we'll have to get working before we can enable the new urlbar functionality.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6b130810bc4
Port URL overflow handling from urlbarBindings.xml to UrlbarInput.jsm. r=standard8

Comment 5

[Noemi Erli[:noemi_erli]]

Backed out changeset a6b130810bc4 (Bug 1491252) for xpcshall failures in browser/components/urlbar/tests/unit/test_UrlbarInput_unit.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed,busted,exception&classifiedState=unclassified&revision=a6b130810bc42f125709fe40c2e26fe9f5f985f6&selectedJob=200771121

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=200771121&repo=mozilla-inbound&lineNumber=10665

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/ec103090a314db31d320c37eaffc5dfa4532cb02

Comment 6

[Dão Gottwald [:dao]]

(In reply to Noemi Erli[:noemi_erli] from comment #5)
> Backed out changeset a6b130810bc4 (Bug 1491252) for xpcshall failures in
> browser/components/urlbar/tests/unit/test_UrlbarInput_unit.js

ERROR Unexpected exception TypeError: this.inputField is undefined; can't access its "addEventListener" property at resource:///modules/UrlbarInput.jsm:79
UrlbarInput@resource:///modules/UrlbarInput.jsm:79:5

*sigh*

UrlbarInput.jsm is very UI focused, so in hindsight it was probably a bad idea to test this with an xpcshell test rather than a mochitest-browser one. For now, I'll just extend the hacks in the test to make it pass.

Comment 7

[Dão Gottwald [:dao]]

Attachment: patch v2

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f138b7d87e7f
Port URL overflow handling from urlbarBindings.xml to UrlbarInput.jsm. r=standard8

Comment 9

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [::dao] from comment #6)
> For now, I'll just extend the hacks in the test to make it pass.

Hacks, or proper external dependency stubbing? Let's make sure we're all on the same page re. this!

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 11

[Mark Banner (:standard8)]

(In reply to Mike de Boer [:mikedeboer] from comment #9)
> (In reply to Dão Gottwald [::dao] from comment #6)
> > For now, I'll just extend the hacks in the test to make it pass.
> 
> Hacks, or proper external dependency stubbing? Let's make sure we're all on
> the same page re. this!

I was also uncertain when I landed the patch that using xpcshell-test was the right thing to do, but I thought I'd try it anyway.

The problem with xpcshell is that we don't have any way to create fake elements or simulate the, that means we have to formulate our own, but that's likely to get harder as we extend the view components.

I think the tests are at the right level - we should generally be doing a lot more actual unit testing than what we do - however I think we should just move these to mochitests. We can then use real or fake elements as inputs, and stub the methods on those where we need to.

I'll file a bug and get this done today.

Comment 12

[Mark Banner (:standard8)]

Filed bug 1493668