Closed
Bug 795831
Opened 12 years ago
Closed 12 years ago
Add test files to litmus-data repository for video controls testing
Categories
(Mozilla QA Graveyard :: Infrastructure, defect)
Mozilla QA Graveyard
Infrastructure
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: AlexLakatos, Assigned: AlexLakatos)
References
Details
Attachments
(1 file, 6 obsolete files)
|
(deleted),
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
We need test files up on mozqa.com in order to make a mozmill test for https://moztrap.mozilla.org/manage/case/266/
For now, I think we should add at least .mov, .webm and .ogv(ogg).
But feel free to suggest otherwise.
|
||
Comment 1•12 years ago
|
||
I wonder how much we should make use of the real websites for testing that the controls are working, or if we should leave this for the manual testing part. Anthony, what do you think?
Assignee: alex.lakatos → nobody
Component: Mozmill Tests → Infrastructure
I think our automation should check against an internal testcase to make sure the "general" case is working. Any top-site testing should be handled manually as they could very well be Tech Evangelism bugs (ie. incorrect server-side implementations).
|
|
||
Comment 3•12 years ago
|
||
Sounds good to me. So lets create basic video files for all the formats we support and basic user controls. I don't think we should host them in mozmill-tests but on mozqa.com. The appropriate mozmill tests will be remote tests then.
|
Assignee | |
Comment 4•12 years ago
|
||
I've already converted .mov, .ogv and .webm files out of sample_video_10s_nosound, patch pending. Do we need more formats?
|
|
||
Comment 5•12 years ago
|
||
We would need at least a Windows Media format. Then we should be fine AFAICS.
|
|
Assignee | |
Comment 6•12 years ago
|
||
Here are .mov, .wmv, .ogv and .webm.
You'll see that the controls differ from file to file. If the control is missing, it means it is not supported or is broken for the respective video type.
Assignee: nobody → alex.lakatos
Attachment #667516 -
Flags: review?(hskupin)
Attachment #667516 -
Flags: review?(dave.hunt)
|
||
Comment 7•12 years ago
|
||
Comment on attachment 667516 [details] [diff] [review]
patch v1.0
Review of attachment 667516 [details] [diff] [review]:
-----------------------------------------------------------------
In general this looks fine, however I haven't been able to test the WMV file.
::: firefox/video/test_ogv_video_nosound.html
@@ +1,5 @@
> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
> +"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
> +<html>
> +<head>
> + <title>OGV video via <object> (silent)</title>
This is not via <object>
::: firefox/video/test_webm_video_nosound.html
@@ +1,5 @@
> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
> +"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
> +<html>
> +<head>
> + <title>WEBM video via <object> (silent)</title>
This is not via <object>
Attachment #667516 -
Flags: review?(dave.hunt) → review-
|
|
||
Comment 8•12 years ago
|
||
Comment on attachment 667516 [details] [diff] [review]
patch v1.0
Review of attachment 667516 [details] [diff] [review]:
-----------------------------------------------------------------
The pages look good as far as I can see. If you can promise that all of those files are working as expected across platforms I would give my r+ on it with the update Dave mentioned.
Attachment #667516 -
Flags: review?(hskupin) → review-
|
|
Assignee | |
Comment 9•12 years ago
|
||
All html files have been touched by this revision of the patch, to ensure they behave the same on all platforms.
Attachment #667516 -
Attachment is obsolete: true
Attachment #670300 -
Flags: review?(hskupin)
Attachment #670300 -
Flags: review?(dave.hunt)
|
|
Assignee | |
Comment 10•12 years ago
|
||
Changed the test_wmv file to correctly handle Flip4Mac and outdated versions of the WMV SDK
Attachment #670300 -
Attachment is obsolete: true
Attachment #670300 -
Flags: review?(hskupin)
Attachment #670300 -
Flags: review?(dave.hunt)
Attachment #670434 -
Flags: review?(hskupin)
Attachment #670434 -
Flags: review?(dave.hunt)
|
|
||
Comment 11•12 years ago
|
||
Comment on attachment 670434 [details] [diff] [review]
patch v3.0
Review of attachment 670434 [details] [diff] [review]:
-----------------------------------------------------------------
::: firefox/video/test_wmv_video_nosound.html
@@ +9,5 @@
> + }
> +
> + function play() {
> + // Flip4Mac uses an older version of the WMV SDK that does not have the controls object
> + if (movie.controls === undefined) {
I wonder if a better approach would be:
if (movie.contols) {
movie.controls.play();
} else {
movie.play();
}
@@ +39,5 @@
> + </embed>
> + <br />
> +
> + <a href="#" onclick="play()" id="play">play</a>
> + <a href="#" onclick="pause()" id="pause">pause</a>
Can we not include seek for WMV too? I just checked and you can get/set the current position:
http://msdn.microsoft.com/en-us/library/ms869378.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/dd562645%28v=vs.85%29.aspx
Attachment #670434 -
Flags: review?(dave.hunt) → review-
|
|
Assignee | |
Comment 12•12 years ago
|
||
> I wonder if a better approach would be:
>
> if (movie.contols) {
> movie.controls.play();
> } else {
> movie.play();
> }
Seemed better, so I went with this, after I fixed the typo :)
> Can we not include seek for WMV too? I just checked and you can get/set the
> current position:
>
> http://msdn.microsoft.com/en-us/library/ms869378.aspx
> http://msdn.microsoft.com/en-us/library/windows/desktop/dd562645%28v=vs.
> 85%29.aspx
The getter works but the setter is not working. May be because I'm using a third party plugin, and not the one from Microsoft.
Attachment #670877 -
Flags: review?(hskupin)
Attachment #670877 -
Flags: review?(dave.hunt)
|
|
||
Updated•12 years ago
|
Attachment #670434 -
Attachment is obsolete: true
Attachment #670434 -
Flags: review?(hskupin)
|
|
||
Comment 13•12 years ago
|
||
(In reply to Alex Lakatos[:AlexLakatos] from comment #12)
> Seemed better, so I went with this, after I fixed the typo :)
D'oh! Thanks! :)
> > Can we not include seek for WMV too? I just checked and you can get/set the
> > current position:
> >
> > http://msdn.microsoft.com/en-us/library/ms869378.aspx
> > http://msdn.microsoft.com/en-us/library/windows/desktop/dd562645%28v=vs.
> > 85%29.aspx
> The getter works but the setter is not working. May be because I'm using a
> third party plugin, and not the one from Microsoft.
Hmm.. it worked for me, what error are you getting?
|
|
Assignee | |
Comment 14•12 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #13)
> > > Can we not include seek for WMV too? I just checked and you can get/set the
> > > current position:
> > >
> > > http://msdn.microsoft.com/en-us/library/ms869378.aspx
> > > http://msdn.microsoft.com/en-us/library/windows/desktop/dd562645%28v=vs.
> > > 85%29.aspx
> > The getter works but the setter is not working. May be because I'm using a
> > third party plugin, and not the one from Microsoft.
>
> Hmm.. it worked for me, what error are you getting?
I'm on Linux and using Totem plugin 3.0.1. Setting the current time does nothing, and I'm not getting any error
|
|
||
Comment 15•12 years ago
|
||
So on Mac I can set position using:
movie.CurrentPosition = "5"
On Ubuntu using Windows Media Player Plug-in 10 (compatible; Totem) I am unable to set the current position, and the following returns false:
movie.controls.isAvailable("currentPosition")
I believe we should have these controls in the HTML files, and it will be down to the tests to use them or not.
|
|
||
Comment 16•12 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #15)
> I believe we should have these controls in the HTML files, and it will be
> down to the tests to use them or not.
Makes sense. If a feature is not available we can hide the control or do what ever so the test itself doesn't run this specific check.
|
|
||
Comment 17•12 years ago
|
||
Comment on attachment 670877 [details] [diff] [review]
patch v4.0
Review of attachment 670877 [details] [diff] [review]:
-----------------------------------------------------------------
Beside what has already been mentioned here a deeper review.
::: firefox/video/test_mov_video_nosound.html
@@ +4,5 @@
> +<head>
> + <title>MOV video via <object> (silent)</title>
> + <script type="text/javascript">
> + function init() {
> + movie = document.getElementById("movie");
nit: add var here.
@@ +21,5 @@
> + }
> +
> + function seek() {
> + var seekTime = document.getElementById("seekField").value;
> + movie.SetTime(parseInt(seekTime));
Beside seeking, is there no event when the current time gets updated? If that's the case please add a comment about it. Also please cache seekTime globally.
::: firefox/video/test_ogv_video_nosound.html
@@ +4,5 @@
> +<head>
> + <title>OGV video via <video> (silent)</title>
> + <script type="text/javascript">
> + function init() {
> + movie = document.getElementById("movie");
nit: add var here.
@@ +6,5 @@
> + <script type="text/javascript">
> + function init() {
> + movie = document.getElementById("movie");
> + movie.addEventListener("timeupdate", function (e) {
> + document.getElementById("currentTime").value = e.target.currentTime;
Please let us cache document.getElementById("currentTime"). Same as for movie.
@@ +19,5 @@
> + movie.pause();
> + }
> +
> + function seek() {
> + var seekTime = document.getElementById("seekField").value;
Please cache.
::: firefox/video/test_webm_video_nosound.html
@@ +4,5 @@
> +<head>
> + <title>WEBM video via <video> (silent)</title>
> + <script type="text/javascript">
> + function init() {
> + movie = document.getElementById("movie");
Add var here.
@@ +6,5 @@
> + <script type="text/javascript">
> + function init() {
> + movie = document.getElementById("movie");
> + movie.addEventListener("timeupdate", function (e) {
> + document.getElementById("currentTime").value = e.target.currentTime;
cache it please.
@@ +19,5 @@
> + movie.pause();
> + }
> +
> + function seek() {
> + var seekTime = document.getElementById("seekField").value;
cache it please
::: firefox/video/test_wmv_video_nosound.html
@@ +4,5 @@
> +<head>
> + <title>WMV video via <embed> (silent)</title>
> + <script type="text/javascript">
> + function init() {
> + movie = document.getElementById("movie");
please add var. Don't we have a timeupdate event?
@@ +39,5 @@
> + </embed>
> + <br />
> +
> + <a href="#" onclick="play()" id="play">play</a>
> + <a href="#" onclick="pause()" id="pause">pause</a>
No seeking possibility available for WMV?
Attachment #670877 -
Flags: review?(hskupin)
Attachment #670877 -
Flags: review?(dave.hunt)
Attachment #670877 -
Flags: review-
|
|
Assignee | |
Comment 18•12 years ago
|
||
movie.controls.isAvailable("currentPosition") will always return false if the movie is not played/playing first.
|
|
||
Comment 19•12 years ago
|
||
(In reply to Alex Lakatos[:AlexLakatos] from comment #18)
> movie.controls.isAvailable("currentPosition") will always return false if
> the movie is not played/playing first.
What platform are you seeing this issue? I would only expect it to work on Windows, however when executing the following in the web console in Firefox 15 on Windows 7 the Windows Media addon immediately crashes.
movie = document.getElementById('movie')
|
|
Assignee | |
Comment 20•12 years ago
|
||
I'm seeing that on Windows.
On linux it always returns false, no matter if it's played before or not. On Mac, there is no isAvailable() function.
You do not need to call movie = document.getElementById('movie') again. Just open the test file, and open the web console. I'm setting movie in the test file already, and it's accessible via the web console. You can just type movie.controls.isAvailable("currentPosition")
|
|
||
Comment 21•12 years ago
|
||
(In reply to Alex Lakatos[:AlexLakatos] from comment #20)
> On linux it always returns false, no matter if it's played before or not.
Right, because it's not available.
> On Mac, there is no isAvailable() function.
Also as I would expect.
> You do not need to call movie = document.getElementById('movie') again. Just
> open the test file, and open the web console. I'm setting movie in the test
> file already, and it's accessible via the web console. You can just type
> movie.controls.isAvailable("currentPosition")
As soon as I reference the movie variable the addon crashes for me.
|
|
Assignee | |
Comment 22•12 years ago
|
||
Is it the correct and latest version of plugin for your sistem?
But does it at least give you an output like true or false in the console?
|
|
||
Comment 23•12 years ago
|
||
Firefox:
Mozilla/5.0 (Windows NT 6.1; rv:16.0) Gecko/20100101 Firefox/16.0
Plugin:
Microsoft® Windows Media Player Firefox Plugin
File: np-mswmp.dll
Version: 1.0.0.8
np-mswmp
|
|
Assignee | |
Comment 24•12 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #21)
> As soon as I reference the movie variable the addon crashes for me.
That's an issue with the web console only. Playing the video from the controls works, as well as referencing the movie object from the Scratchpad.
|
|
Assignee | |
Comment 25•12 years ago
|
||
(In reply to Alex Lakatos[:AlexLakatos] from comment #24)
> (In reply to Dave Hunt (:davehunt) from comment #21)
> > As soon as I reference the movie variable the addon crashes for me.
> That's an issue with the web console only. Playing the video from the
> controls works, as well as referencing the movie object from the Scratchpad.
The issue happens when you access the object via the console by input. So it happens right after you type the dot "movie.". It happens even if you copy/paste the command directly. It does not happen however when you navigate through previous commands or if the command is already in the console input: basically write your command(never mind the crash), refresh the page, run your command.
|
|
Assignee | |
Comment 26•12 years ago
|
||
The crash signature given is already logged, as bug 650788
|
|
||
Comment 27•12 years ago
|
||
I can now replicate the issue with isAvailable("currentPosition"). Searching around, this appears to be a known issue. For now I would suggest including the seek for WMV and degrade gracefully when this option is unavailable. Obviously you can't rely on isAvailable for this.
|
||
Comment 28•12 years ago
|
||
Alex (In reply to Alex Lakatos[:AlexLakatos] from comment #12)
> Created attachment 670877 [details] [diff] [review]
> patch v4.0
>
> > I wonder if a better approach would be:
> >
> > if (movie.contols) {
> > movie.controls.play();
> > } else {
> > movie.play();
> > }
> Seemed better, so I went with this, after I fixed the typo :)
I suggest to have declared before init a new variable:
var movieAction;
in your init function to have:
if (movie.controls) {
movieAction = movie.controls;
} else {
movieAction = movie;
}
and after that for each movie action you just need to call:
movieAction.yourAction();
example:
movieAction.play();
Please check for a better understanding.
http://jsfiddle.net/tWeGa/1/
|
|
||
Comment 29•12 years ago
|
||
As Dave suggested, I will post here suggested code:
//before
function play () {
if (movie.contols) {
movie.controls.play();
} else {
movie.play();
}
}
//after
//instead of this to have in init function
var movieAction;
//....
function init () {
//...
if (movie.controls) {
movieAction = movie.controls;
} else {
movieAction = movie;
}
//...
}
//...
function play() {
movieAction.play();
}
|
|
||
Comment 30•12 years ago
|
||
little rectification for above example is to use else on the next line ( Mozmill Style Guide )
|
|
Assignee | |
Comment 31•12 years ago
|
||
Attachment #684009 -
Flags: review?(hskupin)
|
|
Assignee | |
Updated•12 years ago
|
Attachment #684009 -
Flags: review?(dave.hunt)
|
|
Assignee | |
Updated•12 years ago
|
Attachment #670877 -
Attachment is obsolete: true
|
|
||
Comment 32•12 years ago
|
||
Comment on attachment 684009 [details] [diff] [review]
patch v5.0
Review of attachment 684009 [details] [diff] [review]:
-----------------------------------------------------------------
I would leave the review to Dave given that I have never taken a look into those test cases and it's kinda intensive to check across platforms.
Attachment #684009 -
Flags: review?(hskupin)
|
|
||
Comment 33•12 years ago
|
||
Comment on attachment 684009 [details] [diff] [review]
patch v5.0
Review of attachment 684009 [details] [diff] [review]:
-----------------------------------------------------------------
Most of my comments apply to all files. I'm re-requesting review from Henrik here as I'd like his thoughts. The video files have already been tested, we just need to get the HTML/JavaScript finished up now.
::: firefox/video/test_mov_video_nosound.html
@@ +3,5 @@
> +<html>
> +<head>
> + <title>MOV video via <object> (silent)</title>
> + <script type="text/javascript">
> + var movie, seekTime;
I would prefer these to be within init, and always on separate lines.
@@ +46,5 @@
> + <a href="#" onclick="play()" id="play">play</a>
> + <a href="#" onclick="pause()" id="pause">pause</a>
> + <a href="#" onclick="stop()" id="stop">stop</a>
> + <input type="number" id="seekField" />
> + <a href="#" onclick="seek()" id="seek" value="0">seek</a>
What effect does setting the value to 0 have here?
::: firefox/video/test_wmv_video_nosound.html
@@ +26,5 @@
> + movieAction.pause();
> + }
> +
> + function seek() {
> + // Flip4Mac uses an older version of the WMV SDK that does not have the controls object
Rather than repeat this comment and logic, I would define a movieCurrentPosition in init that is used here.
Attachment #684009 -
Flags: review?(hskupin)
Attachment #684009 -
Flags: review?(dave.hunt)
Attachment #684009 -
Flags: review-
|
|
||
Comment 34•12 years ago
|
||
Comment on attachment 684009 [details] [diff] [review]
patch v5.0
Review of attachment 684009 [details] [diff] [review]:
-----------------------------------------------------------------
Some additional minor nits and a type of leak we have to fix. I also agree with Dave's comments.
::: firefox/video/test_mov_video_nosound.html
@@ +1,2 @@
> +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
> +"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
Please change the doctype so it conforms with HTML 5. We don't want new test files with the old and outdated XHTML document type. The same applies to all the other files in this patch.
@@ +3,5 @@
> +<html>
> +<head>
> + <title>MOV video via <object> (silent)</title>
> + <script type="text/javascript">
> + var movie, seekTime;
It will not work in init. We have to define them here, but I agree that both should be on separate lines.
::: firefox/video/test_ogv_video_nosound.html
@@ +12,5 @@
> + seekTime = document.getElementById("seekField");
> +
> + movie.addEventListener("timeupdate", function (e) {
> + currentTime.value = e.target.currentTime;
> + }, true)
You never unregister the event listener which will cause a leak.
Attachment #684009 -
Flags: review?(hskupin) → review-
|
|
||
Comment 35•12 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #33)
> Comment on attachment 684009 [details] [diff] [review]
> patch v5.0
>
> Review of attachment 684009 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> @@ +46,5 @@
> > + <a href="#" onclick="play()" id="play">play</a>
> > + <a href="#" onclick="pause()" id="pause">pause</a>
> > + <a href="#" onclick="stop()" id="stop">stop</a>
> > + <input type="number" id="seekField" />
> > + <a href="#" onclick="seek()" id="seek" value="0">seek</a>
>
> What effect does setting the value to 0 have here?
If it's not set and seek function is executed it will result in:
movie.SetTime(NaN);
By having value set to "0" we will have always number after parseInt
|
|
||
Comment 36•12 years ago
|
||
Sure? This is on the anchor but not the input field which contains the seek position.
|
|
Assignee | |
Comment 37•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #36)
> Sure? This is on the anchor but not the input field which contains the seek
> position.
That's the issue here, and on the next iteration it will be put on the input tag. That being said, would that be ok with you?
|
|
Assignee | |
Comment 38•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #34)
> ::: firefox/video/test_ogv_video_nosound.html
> @@ +12,5 @@
> > + seekTime = document.getElementById("seekField");
> > +
> > + movie.addEventListener("timeupdate", function (e) {
> > + currentTime.value = e.target.currentTime;
> > + }, true)
>
> You never unregister the event listener which will cause a leak.
So I should unregister with something like window.onunload = teardown; ?
|
|
||
Comment 39•12 years ago
|
||
(In reply to Alex Lakatos[:AlexLakatos] from comment #37)
> (In reply to Henrik Skupin (:whimboo) from comment #36)
> > Sure? This is on the anchor but not the input field which contains the seek
> > position.
>
> That's the issue here, and on the next iteration it will be put on the input
> tag. That being said, would that be ok with you?
Alex please check all files as you set value attribute on "a" element not for input with id "seekField"
|
|
||
Comment 40•12 years ago
|
||
(In reply to Dave Hunt (:davehunt) from comment #33)
> ::: firefox/video/test_wmv_video_nosound.html
> @@ +26,5 @@
> > + movieAction.pause();
> > + }
> > +
> > + function seek() {
> > + // Flip4Mac uses an older version of the WMV SDK that does not have the controls object
>
> Rather than repeat this comment and logic, I would define a
> movieCurrentPosition in init that is used here.
Dave I don't think that this is quite recommended to be done as:
- if movie.controls exists then we need to use: "currentPosition" and if not: "CurrentPosition" (upper "C")
- "currentPosition" needs number, "CurrentPosition" needs string
By adding abstraction to use same logic, our logic will create overhead that is not required.
|
|
Assignee | |
Comment 41•12 years ago
|
||
addressed henrik and dave's review, except the one about movieCurrentPosition
Attachment #684009 -
Attachment is obsolete: true
Attachment #686610 -
Flags: review?(hskupin)
Attachment #686610 -
Flags: review?(dave.hunt)
Attachment #686610 -
Flags: review?(andreea.matei)
|
|
||
Comment 42•12 years ago
|
||
Comment on attachment 686610 [details] [diff] [review]
patch v6.0
Review of attachment 686610 [details] [diff] [review]:
-----------------------------------------------------------------
Just style and comment nits remaining.
::: firefox/video/test_mov_video_nosound.html
@@ +2,5 @@
> +<html>
> +<head>
> + <title>MOV video via <object> (silent)</title>
> + <script type="text/javascript">
> + var movie,
"All variables should be named using camel-case and preceded with the var keyword" so I would expect to see:
var movie;
var seekTime;
If we're to start defining multiple variables in a single line or over several lines with a single var then we should first update our style guide.
::: firefox/video/test_ogv_video_nosound.html
@@ +2,5 @@
> +<html>
> +<head>
> + <title>OGV video via <video> (silent)</title>
> + <script type="text/javascript">
> + var currentTime,
See previous comment
@@ +27,5 @@
> + function seek() {
> + movie.currentTime = parseInt(seekTime.value, 10);
> + }
> +
> + function updateTime(e) {
aEvent
@@ +31,5 @@
> + function updateTime(e) {
> + currentTime.value = e.target.currentTime;
> + }
> +
> + function teardown() {
tearDown
::: firefox/video/test_webm_video_nosound.html
@@ +2,5 @@
> +<html>
> +<head>
> + <title>WEBM video via <video> (silent)</title>
> + <script type="text/javascript">
> + var currentTime,
See previous comment
@@ +27,5 @@
> + function seek() {
> + movie.currentTime = parseInt(seekTime.value, 10);
> + }
> +
> + function updateTime(e) {
aEvent
@@ +31,5 @@
> + function updateTime(e) {
> + currentTime.value = e.target.currentTime;
> + }
> +
> + function teardown() {
tearDown
::: firefox/video/test_wmv_video_nosound.html
@@ +2,5 @@
> +<html>
> +<head>
> + <title>WMV video via <embed> (silent)</title>
> + <script type="text/javascript">
> + var movie,
See previous comment
@@ +27,5 @@
> + movieAction.pause();
> + }
> +
> + function seek() {
> + // Flip4Mac uses an older version of the WMV SDK that does not have the controls object
This comment should reflect the difference in setting the current position rather than the controls object, which is handled in init
Attachment #686610 -
Flags: review?(hskupin)
Attachment #686610 -
Flags: review?(dave.hunt)
Attachment #686610 -
Flags: review?(andreea.matei)
Attachment #686610 -
Flags: review-
|
|
Assignee | |
Comment 43•12 years ago
|
||
used single lines for each variable. adjusted the comment
Attachment #686610 -
Attachment is obsolete: true
Attachment #688315 -
Flags: review?(hskupin)
Attachment #688315 -
Flags: review?(dave.hunt)
Attachment #688315 -
Flags: review?(andreea.matei)
|
|
||
Comment 44•12 years ago
|
||
Comment on attachment 688315 [details] [diff] [review]
patch v7.0
Review of attachment 688315 [details] [diff] [review]:
-----------------------------------------------------------------
Last but not least this looks fine now. I will land it.
Attachment #688315 -
Flags: review?(hskupin)
Attachment #688315 -
Flags: review?(dave.hunt)
Attachment #688315 -
Flags: review?(andreea.matei)
Attachment #688315 -
Flags: review+
|
|
||
Comment 45•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
|
||
Updated•6 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•