892240 - Work - NewUI - Add tests for trimming and formatting of the URI

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: App Bar, defect)

Description

[Jonathan Wilde [:jwilde]]

Currently, we're missing this

Comment 1

[Jonathan Wilde [:jwilde]]

Attachment: tests - depends on all patches in 883390

Probably want to wait on reviewing this until all of the but 883394 patches land, but just to get it up here.

Comment 2

[Jonathan Wilde [:jwilde]]

Attachment: tests - depends on all patches in 883390

Re-exporting. Sorry for the bugspam.

Comment 3

[Jonathan Wilde [:jwilde]]

Attachment: tests

Unbitrotted and should apply against today's m-c.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 776534 [details] [diff] [review]
tests

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

Don't see any issue here and these run fine on my surface pro. Not sure about the urlbar.xml edits though, don't really know that code at all.

::: browser/metro/base/tests/mochitest/browser_urlbar_highlightURLs.js
@@ +47,5 @@
> +  setUp: setUp,
> +  tearDown: tearDown,
> +  run: function () {
> +    let testcases = [
> +      "<https://>mozilla.org",

slick!

Comment 5

[Jonathan Wilde [:jwilde]]

Comment on attachment 776534 [details] [diff] [review]
tests

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

Bringing sfoster on to look at the urlbar.xml stuff. :D

Comment 6

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 776534 [details] [diff] [review]
tests

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

Overall looks great. We talked in channel about the touch/copy behavior - I was not able to copy the full url when I browsed to a page and touched in the urlbar to select, long press on selection and then copy. 

Also, I ran the full testsuite and the test files seperately. In the full test suite run, I got a test failure caused by an "Already Loading" exception in browser_urlbar_trimURLs.js, but when I ran it again it didn't reproduce.

Comment 7

[Jonathan Wilde [:jwilde]]

The touch/copy behavior was fixed in bug 894715. As for the "already loading" exception, it looks like we get that out of more tests than just these, and there may be an issue related to the proxy system in the test runner?

I won't have more cycles to work on this and see whether this is the case this summer, though.

Comment 8

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Tests v2

unbitrotting again. All passed on a couple of local runs. Created a try run:
https://tbpl.mozilla.org/?tree=Try&rev=675b1f5bb7d1

I didn't look into what the urlbar changes are doing though.

Comment 9

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 801928 [details] [diff] [review]
Tests v2

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

Looks good, has a green try run and worked locally on my build also.

::: browser/metro/base/content/bindings/urlbar.xml
@@ +432,5 @@
>        <!-- Entering editing mode -->
>  
> +      <handler event="focus" phase="capturing">
> +        <![CDATA[
> +          this.beginEditing();

Doesn't seem to hurt or help with bug 903866 FWIW, but keeps better track of state within the binding.

Comment 10

[Rodrigo Silveira [:rsilveira] [:rodms]]

https://hg.mozilla.org/integration/fx-team/rev/150bd140c640

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/150bd140c640

Comment 12

[Jim Mathies [:jimm]]

I think we should disable this test unless an easy fix for bug 915620 is apparent.

Comment 13

[Sam Foster [:sfoster] (he/him)]

I'm in the tests today, I'll take a quick look at 915620.