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

RESOLVED FIXED in Firefox 26

Status

Firefox for Metro
App Bar
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jwilde, Assigned: rsilveira)

Tracking

unspecified
Firefox 26
x86_64
Windows 8
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: feature=work)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
Currently, we're missing this
(Reporter)

Updated

5 years ago
Depends on: 883390
Summary: Add tests for trimming and formatting of the URI → Work - NewUI - Add tests for trimming and formatting of the URI
(Reporter)

Updated

5 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 1

5 years ago
Created attachment 774999 [details] [diff] [review]
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.
Attachment #774999 - Flags: review?(jmathies)
(Reporter)

Comment 2

5 years ago
Created attachment 775001 [details] [diff] [review]
tests - depends on all patches in 883390

Re-exporting. Sorry for the bugspam.
Attachment #774999 - Attachment is obsolete: true
Attachment #774999 - Flags: review?(jmathies)
Attachment #775001 - Flags: review?(jmathies)
(Reporter)

Comment 3

5 years ago
Created attachment 776534 [details] [diff] [review]
tests

Unbitrotted and should apply against today's m-c.
Attachment #775001 - Attachment is obsolete: true
Attachment #775001 - Flags: review?(jmathies)
Attachment #776534 - Flags: review?(jmathies)

Comment 4

5 years ago
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!
Attachment #776534 - Flags: review?(jmathies) → review+
(Reporter)

Comment 5

5 years ago
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
Attachment #776534 - Flags: review?(sfoster)
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.
Attachment #776534 - Flags: review?(sfoster) → feedback+
Whiteboard: feature=work → feature=work [preview]
Whiteboard: feature=work [preview] → feature=work [preview-triage]

Updated

5 years ago
Whiteboard: feature=work [preview-triage] → feature=work
(Reporter)

Comment 7

5 years ago
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.
(Reporter)

Updated

5 years ago
Assignee: jwilde → nobody
Created attachment 801928 [details] [diff] [review]
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.
Assignee: nobody → rsilveira
Attachment #776534 - Attachment is obsolete: true
Attachment #801928 - Flags: review?(sfoster)
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.
Attachment #801928 - Flags: review?(sfoster) → review+
https://hg.mozilla.org/mozilla-central/rev/150bd140c640
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26

Comment 12

4 years ago
I think we should disable this test unless an easy fix for bug 915620 is apparent.
Depends on: 915620
I'm in the tests today, I'll take a quick look at 915620.

Updated

4 years ago
No longer depends on: 915620
You need to log in before you can comment on or make changes to this bug.