1188667 - [meta] View Source for 2.5

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] DouglasSherk:1188667-view-source > mozilla-b2g:master

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8648214 [details] [review]
[gaia] DouglasSherk:1188667-view-source > mozilla-b2g:master

How do you test this?

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I mean, automation tests, of course.

Comment 4

[Doug Sherk (:drs) (inactive)]

That's a good question. I'm not sure how we can test add-ons. Fabrice, any suggestions?

Comment 5

[Candice Serran (:cserran)]

Are the dependent bugs on here hard requirements for 2.5? (because they have the 2.5+ feature flag marked and they haven't been resolved)

Comment 6

[[:fabrice] Fabrice Desré]

We talked a bit on irc. On gecko we have mochitests to check that we inject scripts properly, but it's a bit unclear how we can test that in gaia integration tests.

Maybe something like:
frameA.html (the 'driver') includes frameB.html, we trigger view-source on frameB.html, and frameA reaches into frameB to check that the content of the view-source iframe is as expected.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

A Gij test accompany with the patch would be good.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] DouglasSherk:1188667-view-source > mozilla-b2g:master

Comment 9

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8653191 [details] [review]
[gaia] DouglasSherk:1188667-view-source > mozilla-b2g:master

We're no longer using this, instead opting to build the feature into the core OS.

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

FYI, No-Jun might be the tester going forward for this feature.

Comment 11

[Doug Sherk (:drs) (inactive)]

Attachment: View Source window screenshot

Comment 12

[Doug Sherk (:drs) (inactive)]

Attachment: Sleep menu option screenshot

Comment 13

[Doug Sherk (:drs) (inactive)]

Attachment: View Source window screenshot 2

Comment 14

[Doug Sherk (:drs) (inactive)]

Attachment: View Source window screenshot 3

Comment 15

[Doug Sherk (:drs) (inactive)]

Attachment: Settings > Developer Menu option screenshot

Comment 16

[Doug Sherk (:drs) (inactive)]

Attachment: View Source demo video

Please see the other attachments in this bug for screenshots.

Note that we don't have much control over the text styling, colors, etc. as they are all shared with Firefox and come from the 'view-source:' URL scheme. I can, however, change the look of the window, transparency, method to open/close it, etc.

Comment 17

[Jacqueline Savory [:jsavory] UX]

Comment on attachment 8653766 [details]
View Source demo video

Hey Doug, overall it looks good, just a couple things:

- I'm worried how you close the screen isn't obvious enough right now, so instead we could add a header to the screen, titled "View Source" with an 'x' on the left side. Tapping the 'x' would simply close the screen. 

- Its a bit hard to tell in the video, but we could make the screen full screen (if its not already) if we add the header. I think overall it'll look much cleaner that way. 

Let me know if this makes sense or if you have any other questions.

Comment 18

[Amy Lee [:amylee] UX]

Comment on attachment 8653766 [details]
View Source demo video

Just to add to Jacqueline's feedback. I would make the header white with the grey used in settings for the header text and "x" button.

Comment 19

[Amy Lee [:amylee] UX]

Attachment: View_Source.png

Mock for reference to feedback in comment 18.

Comment 20

[Doug Sherk (:drs) (inactive)]

Attachment: View Source screenshot

Thanks for the reviews. I've addressed the issues that you both raised. You can try it out by running attachment 8653198 [details] [review] locally.

Note that I made two changes that deviate from the spec provided in attachment  8655524 [details]:

1. I removed the transparency because I was finding that it was getting in the way of reading.
2. The close button ("X") is on the left instead of the right. This is a convention that we've been following for other apps. It's not trivial to change this as the positioning is built into <gaia-header>, so I'd have to patch that.

Comment 21

[Amy Lee [:amylee] UX]

Comment on attachment 8655905 [details]
View Source screenshot

Hi, 

Can you change the header to white? It looks a bit strange that we're using the Settings header with a white background for the code portion. 

Also please change View source to View Source (capitalized S) to follow our header convention. 

Thanks!

Comment 22

[Amy Lee [:amylee] UX]

Attachment: View_Source.png

Reference mock

Comment 23

[Amy Lee [:amylee] UX]

Attachment: View_Source.png

Comment 24

[Doug Sherk (:drs) (inactive)]

Attachment: View Source screenshot

Fixed based on feedback in comment 21. As before, you can try this feature out by building and flashing attachment 8653198 [details] [review] locally.

Comment 25

[Amy Lee [:amylee] UX]

Comment on attachment 8656708 [details]
View Source screenshot

Hi Doug, 

Looks good to me. Thanks!

Comment 26

[Jacqueline Savory [:jsavory] UX]

Comment on attachment 8656708 [details]
View Source screenshot

Looks good to me as well!